1505689 - Move JSScript::ion and JSScript::baseline to JitScript

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, enhancement, P3)

Comment 1

[Jan de Mooij [:jandem]]

Actually I think we could also move JSScript::baseline to TypeScript. With the raw JIT code pointers in JSScript I don't think we have any hot uses of script->{baseline,ion} left (the ones in BaselineCompiler could just bake in the BaselineScript* directly).

ICScript* could also be stored in TypeScript (or merged in a single class). Maybe renamed to JitScript or something.

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Bug 1505689 part 1 - Tidy up JSScript BaselineScript/IonScript methods. r?tcampbell!

The main goal was to abstract the special BASELINE_DISABLED_SCRIPT and
ION_*_SCRIPT values better. Each of these values now has its own setter
method instead of passing these values to setBaselineScript and setIonScript.

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Bug 1505689 part 2 - Check for ION_DISABLED_SCRIPT in Baseline JIT code. r?tcampbell!

This is more consistent with Baseline Interpreter -> Baseline JIT code and
avoids calling into C++ repeatedly when we disable Ion compilation for the
script after Baseline JIT compilation.

The inIon fix prevents an iloop when running jit-test/tests/ion/bug1324521.js

Depends on D41573

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Bug 1505689 part 3 - Clean up lazy linking code a bit, remove ION_PENDING_SCRIPT. r?tcampbell!

ION_PENDING_SCRIPT didn't add anything over setting |ion| to nullptr.

Depends on D41574

Comment 5

[Jan de Mooij [:jandem]]

Attachment: Bug 1505689 part 4 - Add DisabledBaseline and DisabledIon flags to JSScript. r?tcampbell!

We want to move BaselineScript and IonScript to JitScript, but JitScriptp can be
discarded on GC. These flags ensure this state is persisted when that happens.

Depends on D41575

Comment 6

[Jan de Mooij [:jandem]]

Attachment: Bug 1505689 part 5 - Move BaselineScript and IonScript pointers from JSScript to JitScript. r?tcampbell!

Depends on D41578

Comment 7

[Jan de Mooij [:jandem]]

Attachment: Bug 1505689 part 6 - Merge TraceJitScripts into JitScript::trace. r?tcampbell!

This also removes BaselineScript::Trace and IonScript::Trace that were just
forwarding to the non-static trace().

Depends on D41582

Comment 8

[Jan de Mooij [:jandem]]

Attachment: Bug 1505689 part 7 - Rename DestroyJitScripts to JSScript::releaseJitScriptOnFinalize. r?tcampbell!

This is more consistent with the rest of the code.

Depends on D41708

Comment 9

[Jan de Mooij [:jandem]]

Attachment: Bug 1505689 part 8 - Make JitScript::clear{Baseline,Ion}Script return the cleared script. r?tcampbell!

Depends on D41709

Comment 10

[Jan de Mooij [:jandem]]

Attachment: Bug 1505689 part 9 - Use static const values instead of macros for magic BASELINE_*_SCRIPT and ION_*_SCRIPT values. r?tcampbell!

This defines both |constexpr uintptr_t| and |const pointer| values. reinterpret_cast
in constexpr is not allowed and this way we can still use these values in static_asserts.

Depends on D42108

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b66c924e9a1f
part 1 - Tidy up JSScript BaselineScript/IonScript methods. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/c437f2caa6d0
part 2 - Check for ION_DISABLED_SCRIPT in Baseline JIT code. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/515f936e0df6
part 3 - Clean up lazy linking code a bit, remove ION_PENDING_SCRIPT. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/257f973566ba
part 4 - Add DisabledBaseline and DisabledIon flags to JSScript. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/7db7c0c4fadd
part 5 - Move BaselineScript and IonScript pointers from JSScript to JitScript. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/8bf0e43c0df0
part 6 - Merge TraceJitScripts into JitScript::trace. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/f7a4be5909d3
part 7 - Rename DestroyJitScripts to JSScript::releaseJitScriptOnFinalize. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/11d01998d8f7
part 8 - Make JitScript::clear{Baseline,Ion}Script return the cleared script. r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/d024bd1f20c2
part 9 - Use static const values instead of macros for magic BASELINE_*_SCRIPT and ION_*_SCRIPT values. r=tcampbell

Comment 12

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/b66c924e9a1f
https://hg.mozilla.org/mozilla-central/rev/c437f2caa6d0
https://hg.mozilla.org/mozilla-central/rev/515f936e0df6
https://hg.mozilla.org/mozilla-central/rev/257f973566ba
https://hg.mozilla.org/mozilla-central/rev/7db7c0c4fadd
https://hg.mozilla.org/mozilla-central/rev/8bf0e43c0df0
https://hg.mozilla.org/mozilla-central/rev/f7a4be5909d3
https://hg.mozilla.org/mozilla-central/rev/11d01998d8f7
https://hg.mozilla.org/mozilla-central/rev/d024bd1f20c2

Comment 13

[Alexandru Ionescu :alexandrui]

== Change summary for alert #22490 (as of Fri, 16 Aug 2019 03:55:12 GMT) ==

Improvements:

3% tp5o_webext windows7-32-shippable opt e10s stylo 153.16 -> 149.34
2% tp5o_webext linux64-shippable-qr opt e10s stylo 168.05 -> 164.63

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=22490

Comment 14

[Alexandru Ionescu :alexandrui]

Also:
== Change summary for alert #22484 (as of Fri, 16 Aug 2019 02:45:37 GMT) ==

Improvements:

0.30% Base Content JS macosx1014-64-shippable opt 4,135,234.67 -> 4,123,032.00
0.30% Base Content JS linux64-shippable opt 4,134,152.00 -> 4,121,949.33
0.29% Base Content JS linux64-shippable-qr opt 4,134,232.00 -> 4,122,056.00
0.27% Base Content JS windows10-64-shippable-qr opt 4,191,769.33 -> 4,180,517.33
0.26% Base Content JS windows10-64-shippable opt 4,191,622.67 -> 4,180,517.33

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=22484