Closed
Bug 983486
Opened 8 years ago
Closed 8 years ago
Collect parallel arenas between iterations if data cannot escape
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla31
People
(Reporter: nmatsakis, Assigned: nmatsakis)
References
Details
Attachments
(1 file, 2 obsolete files)
16.67 KB,
patch
|
nmatsakis
:
review+
gkw
:
feedback+
|
Details | Diff | Splinter Review |
During parallel execution the only way that objects can escape a parallel callback is through the return value. Therefore, if the return value is known to have scalar type, no objects can escape, and hence it is reasonable to just completely free all objects in the parallel arenas in between iterations. Given that requesting a GC causes bailouts *and* our allocation routines are not especially parallelized, this turns out to be crucial for performance for applications where the parallel kernel does a lot of allocation. An example is the test case in bug 977567.
Assignee | ||
Comment 1•8 years ago
|
||
I got the bug number wrong of course, it is bug 966567.
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → nmatsakis
Assignee | ||
Comment 2•8 years ago
|
||
Note: a preliminary patch was posted in bug 966567 but I thought it'd be better to just create a separate bug for this work, since there are still other improvements needed for that bug.
Assignee | ||
Comment 3•8 years ago
|
||
Cleaned up version of earlier patch, with feedback applied.
Assignee | ||
Comment 4•8 years ago
|
||
Try run: https://tbpl.mozilla.org/?tree=Try&rev=e453e32af4c1
Assignee | ||
Comment 5•8 years ago
|
||
Attachment #8390932 -
Attachment is obsolete: true
Attachment #8391935 -
Flags: review?(terrence)
Comment 6•8 years ago
|
||
Comment on attachment 8391935 [details] [diff] [review] Bug983486.diff Review of attachment 8391935 [details] [diff] [review]: ----------------------------------------------------------------- Great work! The GC's heap structures and management are a nightmare to understand, much less tweak; this seems pretty reasonable. r=me ::: js/src/jsgc.cpp @@ +545,5 @@ > */ > > + // during parallel sections, we sometimes finalize the parallel arenas, > + // but in that case, we want to hold on to the memory in our arena > + // lists, not offer it up for reuse Start with a capital and end with a period. Also, be sure to change the comment style of the entire method if you want to use a differnet style comment here. ::: js/src/vm/ForkJoin.cpp @@ +1237,5 @@ > > bailouts += 1; > determineBailoutCause(); > > + fprintf(stderr, "recoverFromBailout: %d\n", bailoutCause); I guess you probably want to remove this from the final patch or turn it into SPEW.
Attachment #8391935 -
Flags: review?(terrence) → review+
Assignee | ||
Comment 7•8 years ago
|
||
Comment on attachment 8391935 [details] [diff] [review] Bug983486.diff Requesting fuzzing.
Attachment #8391935 -
Flags: feedback?(gary)
Attachment #8391935 -
Flags: feedback?(choller)
Assignee | ||
Comment 8•8 years ago
|
||
Actually, here is a revised patch with nits applied and fully rebased.
Attachment #8391935 -
Attachment is obsolete: true
Attachment #8391935 -
Flags: feedback?(gary)
Attachment #8391935 -
Flags: feedback?(choller)
Attachment #8392333 -
Flags: review+
Attachment #8392333 -
Flags: feedback?(gary)
Attachment #8392333 -
Flags: feedback?(choller)
Comment on attachment 8392333 [details] [diff] [review] Bug983486.diff Nothing bad found after one overnight run, tested on a 64-bit Linux debug shell.
Attachment #8392333 -
Flags: feedback?(gary) → feedback+
Assignee | ||
Comment 10•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/c02a4b7778cb
Assignee | ||
Comment 11•8 years ago
|
||
Comment on attachment 8392333 [details] [diff] [review] Bug983486.diff Cancelling fuzz request due to limited resources.
Attachment #8392333 -
Flags: feedback?(choller)
Comment 12•8 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/c02a4b7778cb
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in
before you can comment on or make changes to this bug.
Description
•