Bug 151351

Summary: Reserved VM pool established in r187125 is likely too conservative
Product: WebKit Reporter: Michael Saboff <msaboff>
Component: JavaScriptCoreAssignee: Michael Saboff <msaboff>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, keith_miller, mark.lam, sbarati
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Updated patch
fpizlo: review+
Patch for landing none

Description Michael Saboff 2015-11-17 10:39:32 PST
In change set 187125, a 25% reserve of the VM allocation pool was established.  The platform with smallest VM pool is 16MB and a reserve of 25% is 4MB.  Given that the reserve is for "compile must succeed compilations", which are typically for OSR stub generation and other contingent code, this seems a little large.

The proposal is to reduce the reserve from 25% to 15%.

<rdar://problem/22910951>
Comment 1 Michael Saboff 2015-11-17 10:42:15 PST
Created attachment 265680 [details]
Patch
Comment 2 Filip Pizlo 2015-11-17 10:43:59 PST
Comment on attachment 265680 [details]
Patch

I seem to recall 0.25 being necessary to fix a bug.
Comment 3 Michael Saboff 2015-11-17 11:51:52 PST
Created attachment 265688 [details]
Updated patch

After discussing this with Phil and Geoff, we think it would be safer to limit this change to ARM 32 bit only.
Comment 4 Filip Pizlo 2015-11-17 11:56:39 PST
Comment on attachment 265688 [details]
Updated patch

Nice.
Comment 5 Filip Pizlo 2015-11-17 11:57:23 PST
Comment on attachment 265688 [details]
Updated patch

View in context: https://bugs.webkit.org/attachment.cgi?id=265688&action=review

> Source/JavaScriptCore/ChangeLog:8
> +        Reduce the VM allocation reserved pool from 25% to 15% for ARM.

I'd say "ARM32" in the changelog, to make sure that if someone reads this, they don't think you also reduced it for ARM64.
Comment 6 Michael Saboff 2015-11-17 11:58:08 PST
(In reply to comment #5)
> Comment on attachment 265688 [details]
> Updated patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=265688&action=review
> 
> > Source/JavaScriptCore/ChangeLog:8
> > +        Reduce the VM allocation reserved pool from 25% to 15% for ARM.
> 
> I'd say "ARM32" in the changelog, to make sure that if someone reads this,
> they don't think you also reduced it for ARM64.

Will do.
Comment 7 Michael Saboff 2015-11-17 12:30:33 PST
Created attachment 265696 [details]
Patch for landing

Added feedback from prior reviewed patch.
Comment 8 WebKit Commit Bot 2015-11-17 13:30:05 PST
Comment on attachment 265696 [details]
Patch for landing

Clearing flags on attachment: 265696

Committed r192527: <http://trac.webkit.org/changeset/192527>