Bug 195439

Summary: [PlayStation] Increase the default pthread stack size
Product: WebKit Reporter: Christopher Reid <chris.reid>
Component: PlatformAssignee: Nobody <webkit-unassigned>
Status: NEW ---    
Severity: Normal CC: benjamin, cdumez, cmarcelo, dbates, don.olmstead, ews-watchlist, mark.lam, ysuzuki
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch mark.lam: review+, chris.reid: commit-queue?

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 EWS Watchlist 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.