Closed Bug 1855705 Opened 5 months ago Closed 5 months ago

Add a simple cache for Object.assign's fast path

Categories

(Core :: JavaScript Engine, task)

task

Tracking

()

RESOLVED FIXED
120 Branch
Tracking Status
firefox120 --- fixed

People

(Reporter: jandem, Assigned: jandem)

References

(Blocks 1 open bug)

Details

(Whiteboard: [sp3])

Attachments

(2 files)

I looked into this after bug 1853467 and a one-entry cache for the fast path we have for plain objects is highly effective: it has a hit rate > 90% on both Speedometer 2 and 3 and improves a shell version of the micro-benchmark in bug 1853467 as follows:

frozen-10 Took 702.12ms. => 291.76ms.
frozen-8 Took 573.66ms. => 243.06ms.
frozen-200 Took 1565.07ms. => 510.86ms.

regular-10 Took 480.85ms. => 275.58ms.
regular-8 Took 428.57ms. => 247.62ms.
regular-200 Took 1009.22ms. => 476.14ms.

The fast path we have for plain objects only works with data properties stored
in the shape, so a cache based on the shapes of the two objects works really
well for this code.

Even though it's a one-entry cache, the lookup method has a hit rate of
at least 90% on both Speedometer 2 and 3 so it's very effective.

Depends on D189503

Whiteboard: [sp3]
Pushed by jdemooij@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/36cb8b2bc8e2
part 1 - Simplify loop in TryAssignPlain a bit. r=jonco
https://hg.mozilla.org/integration/autoland/rev/26356a7c39e4
part 2 - Add a cache for Object.assign's fast path. r=jonco

Backed out for causing assertion failures on Object.cpp

[task 2023-09-28T16:30:53.221Z] 16:30:53     INFO - GECKO(1564) | must wait for focus in content
[task 2023-09-28T16:30:53.579Z] 16:30:53     INFO - GECKO(1564) | Assertion failure: fromPlain->slotSpan() == props.length(), at /builds/worker/checkouts/gecko/js/src/builtin/Object.cpp:965
[task 2023-09-28T16:30:53.581Z] 16:30:53     INFO -  Initializing stack-fixing for the first stack frame, this may take a while...
[task 2023-09-28T16:31:04.247Z] 16:31:04     INFO - GECKO(1564) | #01: JS_AssignObject(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSObject*>) [js/src/builtin/Object.cpp:1174]
[task 2023-09-28T16:31:04.247Z] 16:31:04     INFO - GECKO(1564) | #02: obj_assign(JSContext*, unsigned int, JS::Value*) [js/src/builtin/Object.cpp:1221]
[task 2023-09-28T16:31:04.247Z] 16:31:04     INFO - GECKO(1564) | #03: CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), js::CallReason, JS::CallArgs const&) [js/src/vm/Interpreter.cpp:486]
[task 2023-09-28T16:31:04.247Z] 16:31:04     INFO - GECKO(1564) | #04: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) [js/src/vm/Interpreter.cpp:580]
[task 2023-09-28T16:31:04.247Z] 16:31:04     INFO - GECKO(1564) | #05: js::Interpret(JSContext*, js::RunState&) [js/src/vm/Interpreter.cpp:0]
[task 2023-09-28T16:31:04.247Z] 16:31:04     INFO - GECKO(1564) | #06: js::RunScript(JSContext*, js::RunState&) [js/src/vm/Interpreter.cpp:458]
[task 2023-09-28T16:31:04.247Z] 16:31:04     INFO - GECKO(1564) | #07: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct, js::CallReason) [js/src/vm/Interpreter.cpp:612]
[task 2023-09-28T16:31:04.247Z] 16:31:04     INFO - GECKO(1564) | #08: js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>, js::CallReason) [js/src/vm/Interpreter.cpp:679]
[task 2023-09-28T16:31:04.247Z] 16:31:04     INFO - GECKO(1564) | #09: js::CallSelfHostedFunction(JSContext*, JS::Handle<js::PropertyName*>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) [js/src/vm/SelfHosting.cpp:1520]
[task 2023-09-28T16:31:04.247Z] 16:31:04     INFO - GECKO(1564) | #10: js::jit::InterpretResume(JSContext*, JS::Handle<JSObject*>, JS::Value*, JS::MutableHandle<JS::Value>) [js/src/jit/VMFunctions.cpp:1027]
[task 2023-09-28T16:31:04.247Z] 16:31:04     INFO - GECKO(1564) | #11: ??? (???:???)
[task 2023-09-28T16:31:04.247Z] 16:31:04     INFO - GECKO(1564) | ExceptionHandler::GenerateDump cloned child 2153
[task 2023-09-28T16:31:04.247Z] 16:31:04     INFO - GECKO(1564) | ExceptionHandler::SendContinueSignalToChild sent continue signal to child
[task 2023-09-28T16:31:04.247Z] 16:31:04     INFO - GECKO(1564) | ExceptionHandler::WaitForContinueSignal waiting for continue signal...
Flags: needinfo?(jdemooij)

Bleh, I think it's just a bogus assertion for dictionary objects. I'll add a test and confirm that's what's going on.

I fixed the assertion and added some tests.

Flags: needinfo?(jdemooij)
Pushed by jdemooij@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/684ff70f0f9c
part 1 - Simplify loop in TryAssignPlain a bit. r=jonco
https://hg.mozilla.org/integration/autoland/rev/7728a6fdad94
part 2 - Add a cache for Object.assign's fast path. r=jonco
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → 120 Branch
Blocks: 1814711
You need to log in before you can comment on or make changes to this bug.