Closed Bug 1571446 Opened 8 months ago Closed 6 months ago

Use a union for jitScript_ and warmUpCount_ fields in JSScript

Categories

(Core :: JavaScript Engine, task, P2)

task

Tracking

()

RESOLVED FIXED
mozilla71
Tracking Status
firefox71 --- fixed

People

(Reporter: jandem, Assigned: jandem)

References

(Blocks 1 open bug)

Details

Attachments

(3 files, 1 obsolete file)

The idea is that the warm-up count would be stored in JitScript, but until the script has a JitScript we can store small values directly in JSScript instead of in JitScript. The JITs and Baseline Interpreter rely on JitScript so they will always use the field in JitScript.

This is working towards the design in bug 1529456 comment 1.

Priority: -- → P2

Stupid remark.

How high can a warm-up counter reach while stored in the JSScript?
Maybe, we should stop incrementing the warm-up counter when running in interpreter-only mode.

(In reply to Nicolas B. Pierron [:nbp] from comment #1)

How high can a warm-up counter reach while stored in the JSScript?
Maybe, we should stop incrementing the warm-up counter when running in interpreter-only mode.

Yeah, I think we would limit it to 1000 or so, well within the page size (4096) and good enough for the C++ interpreter.

maybeJitScript() will become a bit slower in the future but many callers know
statically they have a script with a JitScript so by calling jitScript() there
we can avoid the hasJitScript() check in opt builds.

This is also consistent with the baselineScript() and ionScript() accessors.

I renamed jitScript() to jitScriptX() and then fixed all callers to call
the right method.

This makes it easier to change the jitScript_ field in later patches.

Depends on D42288

Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Keywords: leave-open
Pushed by jdemooij@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/b2089a5f1393
part 1 - Make JSScript::jitScript() assert hasJitScript() and add JSScript::maybeJitScript(). r=tcampbell
https://hg.mozilla.org/integration/autoland/rev/8f72a3e136c8
part 2 - Use accessors instead of accessing jitScript_ directly in various JSScript methods. r=tcampbell

The warm-up count is stored in ScriptWarmUpData until the script is warm
enough for the Baseline Interpreter and the JitScript is created. At that point
we use the warm-up count stored in JitScript.

ScriptWarmUpData uses pointer tagging. This should make it easy to add new
types for LazyScript data in the future.

Keywords: leave-open
Pushed by jdemooij@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e753d23c1237
part 3 - Combine JSScript's jitScript_ and warmUpCount_ fields in a single warmUpData_ field. r=tcampbell
Pushed by nbeleuzu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5921edc9bf59
follow-up - Don't use std::min to avoid weird linker errors in no-opt debug builds. CLOSED TREE
Status: ASSIGNED → RESOLVED
Closed: 6 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71
Attachment #9099833 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.