Closed Bug 1505689 Opened 6 years ago Closed 5 years ago

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

Categories

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

Product:
Core
Component:
JavaScript Engine: JIT
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla70
Tracking Flags:
Tracking Status
firefox70 --- fixed

People

(Reporter: jandem, Assigned: jandem)

References

(Blocks 1 open bug)

Details

Attachments

(9 files)

Bug 1505689 part 1 - Tidy up JSScript BaselineScript/IonScript methods. r?tcampbell!
(deleted), text/x-phabricator-request
Details
Bug 1505689 part 2 - Check for ION_DISABLED_SCRIPT in Baseline JIT code. r?tcampbell!
(deleted), text/x-phabricator-request
Details
Bug 1505689 part 3 - Clean up lazy linking code a bit, remove ION_PENDING_SCRIPT. r?tcampbell!
(deleted), text/x-phabricator-request
Details
Bug 1505689 part 4 - Add DisabledBaseline and DisabledIon flags to JSScript. r?tcampbell!
(deleted), text/x-phabricator-request
Details
Bug 1505689 part 5 - Move BaselineScript and IonScript pointers from JSScript to JitScript. r?tcampbell!
(deleted), text/x-phabricator-request
Details
Bug 1505689 part 6 - Merge TraceJitScripts into JitScript::trace. r?tcampbell!
(deleted), text/x-phabricator-request
Details
Bug 1505689 part 7 - Rename DestroyJitScripts to JSScript::releaseJitScriptOnFinalize. r?tcampbell!
(deleted), text/x-phabricator-request
Details
Bug 1505689 part 8 - Make JitScript::clear{Baseline,Ion}Script return the cleared script. r?tcampbell!
(deleted), text/x-phabricator-request
Details
Bug 1505689 part 9 - Use static const values instead of macros for magic BASELINE_*_SCRIPT and ION_*_SCRIPT values. r?tcampbell!
(deleted), text/x-phabricator-request
Details
No description provided.
URL: http://
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.
Priority: -- → P3
Blocks: 1529456
Summary: Move JSScript::ion to BaselineScript → Move JSScript::ion and JSScript::baseline to JitScript
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED

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.

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

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

Depends on D41574

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

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

Depends on D41582

This is more consistent with the rest of the code.

Depends on D41708

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

Pushed by jdemooij@mozilla.com: 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

== 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

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

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: