Closed Bug 856206 Opened 10 years ago Closed 10 years ago

PJS: Teach TI about transitive compilation

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla23

People

(Reporter: shu, Assigned: shu)

Details

Attachments

(1 file)

If TI causes an invalidation during a transitive compilation in PJS, the invalidated script should be added back to the worklist.
Assignee: general → shu
Attached patch patchSplinter Review
Transitive compilation in compiling a kernel for parallel execution loops until fixed point, i.e. until the entire known call graph of the kernel is compiled. If one of the reachable functions is invalidated due to TI during transitive compilation, we need to add that script back onto the worklist.
Attachment #731406 - Flags: review?(jdemooij)
Attachment #731406 - Flags: review?(jdemooij) → review+
Comment on attachment 731406 [details] [diff] [review]
patch

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

::: js/src/jsinfer.cpp
@@ +2765,5 @@
> +             * the worklist.
> +             */
> +            if (transitiveCompilationWorklist) {
> +                transitiveCompilationWorklist->insert(transitiveCompilationWorklist->begin(),
> +                                                      co.script);

Do we really have to insert at the beginning? At the end seems a bit more efficient because you don't have to shift elements. Also, should we check for duplicates?
(In reply to Jan de Mooij [:jandem] from comment #2)
> Comment on attachment 731406 [details] [diff] [review]
> patch
> 
> Review of attachment 731406 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: js/src/jsinfer.cpp
> @@ +2765,5 @@
> > +             * the worklist.
> > +             */
> > +            if (transitiveCompilationWorklist) {
> > +                transitiveCompilationWorklist->insert(transitiveCompilationWorklist->begin(),
> > +                                                      co.script);
> 
> Do we really have to insert at the beginning? At the end seems a bit more
> efficient because you don't have to shift elements. Also, should we check
> for duplicates?

We're popping off the tail, so logically it makes sense to add the invalidated script onto the 'end' (head) of the worklist queue so it gets processed in the next round. My gut intuition is that most callgraphs will have callers before callees, so appending it at the end might cause some useless recompilations if it gets invalidated again.

Transitive compilation happens rarely enough and the queues are small enough (< 5 on average) that I don't think the efficiency or the checking for duplicates matters.
https://hg.mozilla.org/mozilla-central/rev/5e1117b3ad7f
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla23
You need to log in before you can comment on or make changes to this bug.