972817 - Consider removing the TI flag and enabling it unconditionally

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jan de Mooij [:jandem]]

The prefs/flags we have for the JITs are extremely useful for testing/fuzzing/triage, and we really want to keep them. However, the TI pref isn't all that useful anymore (a lot has been moved into Ion anyway) and I don't think turning off TI noticeably improves memory usage these days.  Also note that we're working on enabling TI and the JITs everywhere (bug 929374, bug 939562).

This would simplify some other parts of the VM as well (for instance, an object's "initialized length" depends on whether TI is enabled). And when we do this we can also remove the --no-ti run from jit_test.py --tbpl, saving resources.

There's one problem I can think of: TI OOMs currently disable TI. I don't know if that's still necessary though.

Brian, what do you think?

Comment 1

[Brian Hackett [Laid off!]]

TI ooms used to disable TI because the constraint graph got into an inconsistent state we couldn't resolve.  The condtraint graph doesn't exist anymore so we should be able to get rid of the type nuking.  Though it would be good if the TI methods generally stayed infallible and handled failure by marking things unknown, canceling any active compilation, etc.

Comment 2

[Brian Hackett [Laid off!]]

Attachment: patch

Comment 3

[Jan de Mooij [:jandem]]

Comment on attachment 8392239 [details] [diff] [review]
patch

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

Awesome!

Please update this comment in ObjectImpl.h as well:

 ... When type inference is disabled,
 * the initialized length always equals the capacity. When inference is
 * enabled, the initialized length is some value less than or equal to both the
 * object's length and the object's capacity.
 *
 * With inference enabled, ...

::: js/src/jsinfer.cpp
@@ -1875,5 @@
> -TypeZone::init(JSContext *cx)
> -{
> -    if (!cx ||
> -        !cx->runtime()->options().typeInference() ||
> -        !cx->runtime()->jitSupportsFloatingPoint)

I think we rely on this to disable Ion compilation if we've no FPU (--no-fpu shell flag should trigger assertion failures on x86). We could add |cx->runtime()->jitSupportsFloatingPoint| to IsIonEnabled.

Comment 4

[Jan de Mooij [:jandem]]

gkw/decoder: this patch will remove the --no-ti and --ti shell flags.

Comment 5

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2044699c3b05

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/2044699c3b05

Comment 7

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to Jan de Mooij [:jandem] from comment #4)
> gkw/decoder: this patch will remove the --no-ti and --ti shell flags.

Jan, thanks for the headsup. I have removed support for --no-ti from the fuzzing harness as of fuzzing rev eb80198dfc4a.