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, saam
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

Michael Saboff
Reported 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>
Attachments
Patch (1.25 KB, patch)
2015-11-17 10:42 PST, Michael Saboff 		no flags
DetailsFormatted DiffDiff
Updated patch (1.29 KB, patch)
2015-11-17 11:51 PST, Michael Saboff 		fpizlo: review+
DetailsFormatted DiffDiff
Patch for landing (1.25 KB, patch)
2015-11-17 12:30 PST, Michael Saboff 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Michael Saboff
Comment 1 2015-11-17 10:42:15 PST
Created attachment 265680 [details] Patch
Filip Pizlo
Comment 2 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.
Michael Saboff
Comment 3 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.
Filip Pizlo
Comment 4 2015-11-17 11:56:39 PST
Comment on attachment 265688 [details] Updated patch Nice.
Filip Pizlo
Comment 5 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.
Michael Saboff
Comment 6 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.
Michael Saboff
Comment 7 2015-11-17 12:30:33 PST
Created attachment 265696 [details] Patch for landing Added feedback from prior reviewed patch.
WebKit Commit Bot
Comment 8 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>
Note You need to log in before you can comment on or make changes to this bug.