Bug 195439 - [PlayStation] Increase the default pthread stack size
Summary: [PlayStation] Increase the default pthread stack size
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Platform (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-03-07 15:56 PST by Christopher Reid
Modified: 2019-03-07 17:51 PST (History)
8 users (show)

See Also:


Attachments
patch (1.50 KB, patch)
2019-03-07 16:18 PST, Christopher Reid
mark.lam: review+
chris.reid: commit-queue?
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Christopher Reid 2019-03-07 15:56:00 PST
Our PlayStation platform has a very small stack size of 64 KB for new threads. With default jsc VM options, web workers would instantly stack overflow on a 64 KB stack.
The main thread stack size is more reasonable at 2 MB.
This has came up before with https://bugs.webkit.org/show_bug.cgi?id=40103.

I tested with these jsc VM options:
maxPerThreadStackUsage = 64 * KB
softReservedZoneSize = 32 * KB
reservedZoneSize = 16 * KB

On linux with those options and `ulimit -s 64`, I ran javascriptcore-tests --memory-limited and there are still about 1,000 failures out of 55,000 tests.
There's 67 failures using `ulimit -s 512` and default VM options. 512 KB seems to also match the secondary thread stack size on iOS and Mac.
Comment 1 Christopher Reid 2019-03-07 16:18:37 PST
Created attachment 363951 [details]
patch
Comment 2 Build Bot 2019-03-07 16:20:51 PST
Attachment 363951 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/posix/ThreadingPOSIX.cpp:214:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Mark Lam 2019-03-07 16:35:01 PST
Comment on attachment 363951 [details]
patch

LGTM but someone with PlayStation knowledge should also review to confirm if 512K is a reasonable stack size to use.
Comment 4 Don Olmstead 2019-03-07 17:43:05 PST
(In reply to Mark Lam from comment #3)
> Comment on attachment 363951 [details]
> patch
> 
> LGTM but someone with PlayStation knowledge should also review to confirm if
> 512K is a reasonable stack size to use.

I'm fine with the change. The thing I'm wondering is whether it would be good for WebKit to have a thread's stack size be more configurable in general during creation.
Comment 5 Mark Lam 2019-03-07 17:49:49 PST
(In reply to Don Olmstead from comment #4)
> (In reply to Mark Lam from comment #3)
> > Comment on attachment 363951 [details]
> > patch
> > 
> > LGTM but someone with PlayStation knowledge should also review to confirm if
> > 512K is a reasonable stack size to use.
> 
> I'm fine with the change. The thing I'm wondering is whether it would be
> good for WebKit to have a thread's stack size be more configurable in
> general during creation.

See Options::maxPerThreadStackUsage().  For tests, we explicitly set this option to a smaller amount do that tests that overflow the stack can finish faster.

However, this option is at the JSC level, not WTF.  Actual allowed stack capacity is computed as min of Options::maxPerThreadStackUsage() and capacity reported by StackBounds.
Comment 6 Mark Lam 2019-03-07 17:51:13 PST
(In reply to Mark Lam from comment #5)
> (In reply to Don Olmstead from comment #4)
> > (In reply to Mark Lam from comment #3)
> > > Comment on attachment 363951 [details]
> > > patch
> > > 
> > > LGTM but someone with PlayStation knowledge should also review to confirm if
> > > 512K is a reasonable stack size to use.
> > 
> > I'm fine with the change. The thing I'm wondering is whether it would be
> > good for WebKit to have a thread's stack size be more configurable in
> > general during creation.
> 
> See Options::maxPerThreadStackUsage().  For tests, we explicitly set this
> option to a smaller amount do that tests that overflow the stack can finish
> faster.
> 
> However, this option is at the JSC level, not WTF.  Actual allowed stack
> capacity is computed as min of Options::maxPerThreadStackUsage() and
> capacity reported by StackBounds.

Note: it's done this way because we can't always control what the OS allocates for stack capacity.  We just make sure that we don't use more than either limits.