Bug 151351 - Reserved VM pool established in r187125 is likely too conservative
Summary: Reserved VM pool established in r187125 is likely too conservative
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Michael Saboff
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2015-11-17 10:39 PST by Michael Saboff
Modified: 2015-11-17 13:38 PST (History)
4 users (show)

See Also:


Attachments
Patch (1.25 KB, patch)
2015-11-17 10:42 PST, Michael Saboff
no flags Details | Formatted Diff | Diff
Updated patch (1.29 KB, patch)
2015-11-17 11:51 PST, Michael Saboff
fpizlo: review+
Details | Formatted Diff | Diff
Patch for landing (1.25 KB, patch)
2015-11-17 12:30 PST, Michael Saboff
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>