Closed Bug 1407607 Opened 2 years ago Closed 2 years ago

Use a single entry point for C++ -> JIT calls

Categories

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

enhancement

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox57 --- wontfix
firefox58 --- fixed

People

(Reporter: jandem, Assigned: jandem)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

In RunScript we first check if Ion is enabled, then we call CanEnter, then we call IonCannon. If that didn't succeed, we try the same things for Baseline.

I think it would be much nicer/faster/simpler to have a single MaybeEnterJit entry point that works more like JIT -> JIT calls: if script->baselineOrIonRaw is non-null we only have to check numActualArgs and then we can just push the JitActivation etc and call the EnterJit trampoline. Having a unified JIT entry point means we can also get rid of EnterJitData/SetEnterJitData overhead.

The OSR path (Interpreter -> Baseline) should probably stay separate as it's petty different from normal script entry, but that code can also be simplified once it's only used for OSR.
Priority: -- → P3
Attached patch PatchSplinter Review
This implements what I described in comment 0 and works well.

The micro-benchmark below improves from 240-250 ms to 200-210 ms so it's a pretty big win.

There's more we can improve in the Baseline OSR code (get rid of EnterJitData) but we can do that as follow-up.

function f() {
    var g = (r) => void 0;
    var count = 1000000;
    var start = Date.now();
    for (var i = 0; i < count; ++i) {
        new Promise(g);
    }
    var stop = Date.now();
    print(stop - start);
}
f();
Attachment #8917445 - Flags: review?(nicolas.b.pierron)
Comment on attachment 8917445 [details] [diff] [review]
Patch

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

::: js/src/jit/Jit.cpp
@@ +47,5 @@
> +
> +        if (TooManyActualArguments(numActualArgs)) {
> +            // Too many arguments for Ion. Baseline supports more actual
> +            // arguments, so in that case force Baseline code. TODO: try to
> +            // unify this.

nit: TODO (Bug ABCDEFG)?

@@ +130,5 @@
> +    return EnterJitStatus::Ok;
> +}
> +
> +EnterJitStatus
> +js::jit::MaybeEnterJit(JSContext* cx, RunState& state)

Nice addition!

::: js/src/jit/Jit.h
@@ +16,5 @@
> +namespace jit {
> +
> +enum class EnterJitStatus
> +{
> +    Error,

nit: Add a comment saying that an exception is expected to be in the JSContext in such case.
Attachment #8917445 - Flags: review?(nicolas.b.pierron) → review+
Pushed by jandemooij@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b1204c61ba56
Use a single entry point for C++ -> JIT calls. r=nbp
Depends on: 1408766
Depends on: 1410823
Depends on: 1441012
No longer depends on: 1441012
Depends on: 1449950
You need to log in before you can comment on or make changes to this bug.