856206 - PJS: Teach TI about transitive compilation

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Shu-yu Guo [:shu]]

If TI causes an invalidation during a transitive compilation in PJS, the invalidated script should be added back to the worklist.

Comment 1

[Shu-yu Guo [:shu]]

Attachment: patch

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.

Comment 2

[Jan de Mooij [:jandem]]

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?

Comment 3

[Shu-yu Guo [:shu]]

(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.

Comment 4

[Shu-yu Guo [:shu]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5e1117b3ad7f

try: https://tbpl.mozilla.org/?tree=Try&rev=7fc6d979bc2c

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/5e1117b3ad7f