Last Comment Bug 706914 - JM: Compile large scripts in chunks
: JM: Compile large scripts in chunks
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal with 4 votes (vote)
: mozilla12
Assigned To: Brian Hackett (:bhackett)
:
:
Mentors:
: 719189 (view as bug list)
Depends on: 712090 712171 712265 712266 712267 719674 719918 720234 720359 721284 722598 726799 727223 728342 728372 728509 730806 732776 770089 817475
Blocks: 689284 670885 687127 716255
  Show dependency treegraph
 
Reported: 2011-12-01 11:18 PST by Brian Hackett (:bhackett)
Modified: 2012-12-02 19:41 PST (History)
20 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
WIP (c4832f2d9986) (223.37 KB, patch)
2011-12-01 11:18 PST, Brian Hackett (:bhackett)
no flags Details | Diff | Splinter Review
WIP (c4832f2d9986) (36.70 KB, patch)
2011-12-01 15:11 PST, Brian Hackett (:bhackett)
no flags Details | Diff | Splinter Review
WIP (228.79 KB, patch)
2011-12-01 17:16 PST, Brian Hackett (:bhackett)
no flags Details | Diff | Splinter Review
patch (c4832f2d9986) (240.25 KB, patch)
2011-12-16 12:03 PST, Brian Hackett (:bhackett)
no flags Details | Diff | Splinter Review
patch (61a46d539c69) (228.13 KB, patch)
2011-12-16 13:54 PST, Brian Hackett (:bhackett)
no flags Details | Diff | Splinter Review
patch (b121a045b451) (231.17 KB, patch)
2011-12-19 09:43 PST, Brian Hackett (:bhackett)
no flags Details | Diff | Splinter Review
patch (b121a045b451) (233.96 KB, patch)
2011-12-21 08:28 PST, Brian Hackett (:bhackett)
dvander: review+
Details | Diff | Splinter Review

Description Brian Hackett (:bhackett) 2011-12-01 11:18:33 PST
Created attachment 578331 [details] [diff] [review]
WIP (c4832f2d9986)

There's a longstanding problem with JM+TI (and any engine that combines both whole method compilation and invalidation/recompilation) where too much time is spent compiling large scripts.  The amount of time to compile a script is linear in its size (optimistic, but seems to hold in practice) and the number of times a script needs to be recompiled is also expected to be linear in its size, all else being equal.  This leads to quadratic behavior which really hurts when dealing with large scripts.

This can be helped (not sure if fixed) by not doing whole method compilation, but rather compiling scripts in chunks.  Pick an arbitrary partitioning of the script into chunks, only compile one chunk at a time and keep track of cross-chunk edges to fix up after doing a compilation or discarding a chunk due to a recompilation.  The attached patch (WIP) does this, still needs more work to not be broken.
Comment 1 Brian Hackett (:bhackett) 2011-12-01 15:11:35 PST
Created attachment 578420 [details] [diff] [review]
WIP (c4832f2d9986)

Passes jit-tests -mna with a chunk size of 10 ops (for stress testing).
Comment 2 Brian Hackett (:bhackett) 2011-12-01 17:16:12 PST
Created attachment 578447 [details] [diff] [review]
WIP

Wrong patch attached earlier.
Comment 3 Brian Hackett (:bhackett) 2011-12-16 12:03:45 PST
Created attachment 582350 [details] [diff] [review]
patch (c4832f2d9986)

Working patch, needs a rebase.
Comment 4 Brian Hackett (:bhackett) 2011-12-16 13:54:30 PST
Created attachment 582383 [details] [diff] [review]
patch (61a46d539c69)

Patch rebased to tip.  The CHUNK_LIMIT environment variable is in place for testing, when this lands it will be hardcoded to something else (like 1000).
Comment 5 Brian Hackett (:bhackett) 2011-12-19 08:11:37 PST
Decoder, Gary: can you guys fuzz this patch some?  It is a significant method JIT change.  Beforehand, you should set the CHUNK_LIMIT environment variable to some smallish value, between 5 and 100 say (this controls how scripts are partitioned during compilation).  The testing should mainly be done on x86, as this functionality is totally disabled on x64 (for now at least, and possibly for the lifetime of JM).  I can also make the chunk limit controllable from the shell with a chunkLimit(N) function if that would make things easier, and will probably take this route anyways when landing the patch.
Comment 6 Brian Hackett (:bhackett) 2011-12-19 09:43:01 PST
Created attachment 582860 [details] [diff] [review]
patch (b121a045b451)

Rebased, removes CHUNK_LIMIT environment variable and allows the chunk limit to be changed in the shell with mjitChunkLimit(N).
Comment 7 Gary Kwong [:gkw] [:nth10sd] 2011-12-19 10:27:31 PST
> least, and possibly for the lifetime of JM).  I can also make the chunk
> limit controllable from the shell with a chunkLimit(N) function if that
> would make things easier, and will probably take this route anyways when
> landing the patch.

Adding the chunkLimit(N) function will likely be really useful. :)
Comment 8 Christian Holler (:decoder) 2011-12-19 10:37:47 PST
I'm on it, will file bugs blocking this one when I have results.
Comment 9 Brian Hackett (:bhackett) 2011-12-21 08:28:36 PST
Created attachment 583509 [details] [diff] [review]
patch (b121a045b451)

Updated with fixes for dependent bugs.
Comment 10 Gary Kwong [:gkw] [:nth10sd] 2012-01-01 02:24:30 PST
> Created attachment 583509 [details] [diff] [review]
> patch (b121a045b451)
> 
> Updated with fixes for dependent bugs.

fwiw, this patch has bitrotted, it would be nice to have a rebase. :)
Comment 11 David Anderson [:dvander] 2012-01-13 13:58:27 PST
Comment on attachment 583509 [details] [diff] [review]
patch (b121a045b451)

Review of attachment 583509 [details] [diff] [review]:
-----------------------------------------------------------------

There's not a whole lot I can do to review this patch but I do have one suggestion with regards to IonMonkey compatibility:

::: js/src/jsinfer.h
@@ +1125,5 @@
> +struct RecompileInfo
> +{
> +    JSScript *script;
> +    bool constructing:1;
> +    uint32_t chunkIndex:31;

It would be much nicer if this structure could be:
    JSScript *script;
    jsbytecode *pc;

And then have processPendingRecompiles do the work of finding the chunk index in the ctor and non-ctor JITScripts. Currently, IonMonkey doesn't do chunked compilation (and won't in the next few months); doesn't have constructor bifurcation; it might end up having a different chunking approach too. So having a more generic interface would be ideal.
Comment 12 Brian Hackett (:bhackett) 2012-01-18 16:44:37 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/d0c192e5bd41
Comment 13 Brian Hackett (:bhackett) 2012-01-18 16:45:48 PST
Oh, for integrating this into IM, just use constructing == false and chunkIndex == 0.  The script/constructing/chunkIndex are just identifiers for a compilation unit, and JM/IM do not track dependencies at such a fine granularity as the pc.
Comment 14 Marco Bonardo [::mak] 2012-01-19 02:57:33 PST
https://hg.mozilla.org/mozilla-central/rev/d0c192e5bd41
Comment 15 David Mandelin [:dmandelin] 2012-01-20 12:06:16 PST
*** Bug 719189 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.