608746 - JM: TypeInference: recompile scripts on dynamic type changes

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Brian Hackett [Laid off!]]

Type inference (bug 557407) is a hybrid between a static and dynamic analysis, and inferred types can change as code is running.  For JM integration, this requires recompiling scripts which are active in the context (either in the current stack frame or somewhere deeper), and fixing up the stack to correct stored values and return value addresses.

Comment 1

[Brian Hackett [Laid off!]]

Attachment: kludge allowing testing without recompilation

This patch allows JM to see the final types without any dynamic changes, by first running the interpreter to pick up those types and *then* running JM on a clone of the original global object.  This lets other optimization work (aka the fun stuff) proceed.

Landing this on Jaegermonkey, but will be easy to back out once recompilation works.

http://hg.mozilla.org/projects/jaegermonkey/rev/a2e6ee94f4bf

Comment 2

[Brian Hackett [Laid off!]]

Attachment: freeze interface cleanup

Patch removing the cobbled together approximation of which code requires recompilation, and fixes the Compiler so that any type information it queries will trigger a recompilation if that information changes.

http://hg.mozilla.org/projects/jaegermonkey/rev/4744eea6dd30

Comment 3

[Brian Hackett [Laid off!]]

Attachment: Monitoring fixes for non-name opcodes

This fixes JM to detect most dynamic type changes that can induce recompilation: arithmetic overflow, reads of holes out of arrays or other objects, writes to unknown objects and calls to unknown functions.  Excepted are reads of undefined globals, upvars or generic names, which will get fixed after or alongside recompilation for other stuff.

http://hg.mozilla.org/projects/jaegermonkey/rev/41824ecd79e1

Comment 4

[Brian Hackett [Laid off!]]

Attachment: WIP

This interfaces with and modifies the debug-mode recompiler to also recompile for dynamic type changes.  Doesn't yet handle recompilation from within an IC'ed native call, or flushing ICs that refer to recompiled scripts.

Comment 5

[Brian Hackett [Laid off!]]

Attachment: WIP 2

This patch fixes up native call ICs which trigger recompilation by moving them from the old to the new JITScript, and does targeted flushing of caller ICs by keeping a list on each JITScript of its IC callers.

Comment 6

[Brian Hackett [Laid off!]]

Attachment: Monitoring fixes for name opcodes

This monitors accesses on undefined names where necessary, which requires undefined checks on NAME opcodes and on GLOBAL opcodes accessing globals not already known to be defined.

http://hg.mozilla.org/projects/jaegermonkey/rev/0d8f3cec719c

Comment 7

[Brian Hackett [Laid off!]]

Attachment: patch

This fixes recompilation to work on all trace-tests except 2.  One needs GC to work (next TODO), the other is due to bug 584603.

http://hg.mozilla.org/projects/jaegermonkey/rev/633c888a78ca

Comment 8

[Brian Hackett [Laid off!]]

Attachment: Analysis and recompilation tuning for SS

This adds some analysis and tuning to cut the number of recompilations on SS.  This is currently at 16; most of the extra ones over numbers in bug 557407 are due to trying to keep keep the result of integer division as an integer.  Several of these can be simply eliminated, e.g. a call Math.floor(i/4) recompiles when i/4 produces a double.  Changes:

- Marking integer division as producing a double when the rhs is not a constant or a local recently assigned a constant.
- Marking arrays as not packed if they were created with [], Array() or Array(N) and have not been initialized with a zero index or local which could be zero (targeting 'for (i = 0; ...; i++) { x[i] = ... }' and similar loops).

With the extra cost of recompiling, extra monitoring cost watching for undefined globals (this can also be reduced greatly) and no goofy run-the-interpreter first behavior, I get a 10% improvement for -m running tests individually (does not include analysis time), 5% running the SS harness (includes analysis time, unlike the SS website).

V8 not tuned, except for v8-crypto (just required fixing Math.pow so that pow(int,int) is expected to be an int).  I get these times on v8-crypto:

v8: 108ms
jsc: 126ms
-m: 98ms
-j: 118ms
-m -j -p: 128ms (bug 608351 I guess)
-m (new): 62ms

Comment 9

[Brian Hackett [Laid off!]]

er, bug 603851

Comment 10

[Brian Hackett [Laid off!]]

http://hg.mozilla.org/projects/jaegermonkey/rev/d86864d9ebac

Comment 11

[David Mandelin [:dmandelin]]

(In reply to comment #8)
> I get these times on v8-crypto:
> v8: 108ms
> jsc: 126ms
> -m: 98ms
> -j: 118ms
> -m -j -p: 128ms (bug 608351 I guess)
> -m (new): 62ms

Very nice. I can't remember if I've said this 0, 1, or \aleph_0 times before, so I'll just say it again anyway: I wouldn't worry about SunSpider results too much beyond avoiding regressions. The tests are so short-running that advanced compiler optimizations are unlikely to pay off. Also, I expect SunSpider will become less relevant as time goes on, in favor of longer-running benchmarks that do interesting computational things. So, in terms of benchmarks, Kraken and V8 seem the most interesting to attack with type inference. Maybe also an app or two like Quake 2.

Comment 12

[Brian Hackett [Laid off!]]

Attachment: GETELEM monitoring fix

Bugfix for monitoring results of GETELEM.  When accessing a dense array we assume that non-string primitive indexes are integers, even if we inferred they could be undefined/null/bool/double (optimistically treating the former three as not actually occurring, and the latter as accessing an integer index).  The GETELEM stub was not checking for this properly.

http://hg.mozilla.org/projects/jaegermonkey/rev/6a3dfe79bfa6