Win64 TEST-UNEXPECTED-FAIL | chrome://mochitests/content/chrome/js/xpconnect/tests/chrome/test_bug732665.xul | Chrome should be able to have at least 10 heavy frames more stack than content: 139, 131

RESOLVED FIXED in mozilla33

Status

()

Core
XPConnect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: dmajor, Assigned: dmajor)

Tracking

32 Branch
mozilla33
x86_64
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
https://tbpl.mozilla.org/php/getParsedLog.php?id=41405451&tree=Date

I haven't looked very deeply, but I'm guessing we just need to add a case for win64 to XPCJSRuntime::XPCJSRuntime.
(Assignee)

Comment 1

4 years ago
How would I measure the right sizes to use for win64?
Flags: needinfo?(bobbyholley)
(In reply to David Major [:dmajor] (UTC+12) from comment #1)
> How would I measure the right sizes to use for win64?

The complete stack size is assumed to be already fixed (I don't know what considerations go into setting it for a given platform).

Once you have that, just run the nearNativeStackLimit function in the test case to figure out how many "heavy" frames it takes to exhaust the stack. That tells you how big a heavy frame is on your platform, and you can then figure out how much buffer space you need to allow 10 extra heavy frames for chrome.

My experience suggests that frames tend to get larger over time, so I'd suggest leaving a bit of an extra margin to avoid having to constantly re-adjust the test.
Flags: needinfo?(bobbyholley)
(Assignee)

Comment 3

4 years ago
Is it just me or are all these statements off by a factor of 2? Am I missing something?

>    // Our "default" stack is what we use in configurations where we don't have
>    // a compelling reason to do things differently. This is effectively 1MB on
>    // 32-bit platforms and 2MB on 64-bit platforms.
>    const size_t kDefaultStackQuota = 128 * sizeof(size_t) * 1024;

>    // ASan requires more stack space due to red-zones, so give it double the
>    // default (2MB on 32-bit, 4MB on 64-bit). ASAN stack frame measurements

>    // 1MB is the default stack size on Windows, so the default 1MB stack quota
>    // we'd get on win32 is slightly too large. Use 900k instead. And since
(Assignee)

Comment 4

4 years ago
Created attachment 8438922 [details] [diff] [review]
Adjust the trusted script buffer size for win64

On my machine, debug builds needed 31k on Win32 and 72k on Win64 (not sure why it's more-than-double)

Shall we just say 48/96?
Attachment #8438922 - Flags: review?(bobbyholley)
Comment on attachment 8438922 [details] [diff] [review]
Adjust the trusted script buffer size for win64

Review of attachment 8438922 [details] [diff] [review]:
-----------------------------------------------------------------

> >    // Our "default" stack is what we use in configurations where we don't have
> >    // a compelling reason to do things differently. This is effectively 1MB on
> >    // 32-bit platforms and 2MB on 64-bit platforms.
> >    const size_t kDefaultStackQuota = 128 * sizeof(size_t) * 1024;
> 
> >    // ASan requires more stack space due to red-zones, so give it double the
> >    // default (2MB on 32-bit, 4MB on 64-bit). ASAN stack frame measurements
> 
> >    // 1MB is the default stack size on Windows, so the default 1MB stack quota
> >    // we'd get on win32 is slightly too large. Use 900k instead. And since

Yeah I think they are. Can you fix the comments?
Attachment #8438922 - Flags: review?(bobbyholley) → review+
https://hg.mozilla.org/mozilla-central/rev/26c6eab210b3
https://hg.mozilla.org/mozilla-central/rev/6f0fe4136952
Assignee: nobody → dmajor
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
You need to log in before you can comment on or make changes to this bug.