Closed Bug 1983154 Opened 1 year ago Closed 11 months ago

Prototype a JS Engine managed MicroTask Queue

Tracking

()

Status:

RESOLVED FIXED

Milestone:

145 Branch

Tracking Flags:

Tracking

Status

firefox145

---

fixed

People

(Reporter: mgaudet, Assigned: mgaudet)

References

(Depends on 1 open bug, Blocks 2 open bugs)

Details

Attachments

(11 files, 2 obsolete files)

WIP: Bug 1983154 - Implement GCFifo 1 year ago Matthew Gaudet (he/him) [:mgaudet] 48 bytes, text/x-phabricator-request		Details \| Review
Bug 1983154 - Move ReactionRecord slots inside of class r?arai 1 year ago Matthew Gaudet (he/him) [:mgaudet] 48 bytes, text/x-phabricator-request		Details \| Review
Bug 1983154 - Split CallSetup out of CallbackObjectBase r?smaug 1 year ago Matthew Gaudet (he/him) [:mgaudet] 48 bytes, text/x-phabricator-request		Details \| Review
Bug 1983154 - Add CallSetup that doesn't take a CallbackObjectBase r?smaug 1 year ago Matthew Gaudet (he/him) [:mgaudet] 48 bytes, text/x-phabricator-request		Details \| Review
Bug 1983154 - Add javascript.options.use_js_microtask_queue r?arai 1 year ago Matthew Gaudet (he/him) [:mgaudet] 48 bytes, text/x-phabricator-request		Details \| Review
Bug 1983154 - Create MaybeGetPromiseAllocationSite r?arai 1 year ago Matthew Gaudet (he/him) [:mgaudet] 48 bytes, text/x-phabricator-request		Details \| Review
Bug 1983154 - Shell support for a JS MicroTaskQueue. r?arai 1 year ago Matthew Gaudet (he/him) [:mgaudet] 48 bytes, text/x-phabricator-request		Details \| Review
Bug 1983154 - Gecko Support for JS Micro Task Queue r?arai!,smaug! 1 year ago Matthew Gaudet (he/him) [:mgaudet] 48 bytes, text/x-phabricator-request		Details \| Review
Bug 1983154 - Make Vector a parameter to ds/fifo.h r?jonco 11 months ago Matthew Gaudet (he/him) [:mgaudet] 48 bytes, text/x-phabricator-request		Details \| Review
Bug 1983154 - Support JS::Value in TraceableFifo r?jonco 11 months ago Matthew Gaudet (he/him) [:mgaudet] 48 bytes, text/x-phabricator-request		Details \| Review
Bug 1983154 - Select debugger queue where required r?arai!,smaug! 11 months ago Matthew Gaudet (he/him) [:mgaudet] 48 bytes, text/x-phabricator-request		Details \| Review
Bug 1983154 - Track the correct execution global more carefully. r?arai! 11 months ago Matthew Gaudet (he/him) [:mgaudet] 48 bytes, text/x-phabricator-request		Details \| Review
Bug 1983154 - Refactor CallSetup r?smaug 11 months ago Matthew Gaudet (he/him) [:mgaudet] 48 bytes, text/x-phabricator-request		Details \| Review

Matthew Gaudet (he/him) [:mgaudet]

Assignee

Description

•

1 year ago

A prototype of a JS engine managed microtask queue shows impressive performance improvement on doxbee-promise, a promise heavy microbenchmark from the JetStream 3.

This bug is to land a prototype disabled, to allow further evolution and evaluation before making it the default.

Flags: behind-pref+

Matthew Gaudet (he/him) [:mgaudet]

Assignee

Comment 1

•

1 year ago

Attached file WIP: Bug 1983154 - Implement GCFifo (obsolete) — Details

Matthew Gaudet (he/him) [:mgaudet]

Assignee

Comment 2

•

1 year ago

Attached file Bug 1983154 - Move ReactionRecord slots inside of class r?arai — Details

Matthew Gaudet (he/him) [:mgaudet]

Assignee

Comment 3

•

1 year ago

Attached file Bug 1983154 - Split CallSetup out of CallbackObjectBase r?smaug — Details

Matthew Gaudet (he/him) [:mgaudet]

Assignee

Comment 4

•

1 year ago

Attached file Bug 1983154 - Add CallSetup that doesn't take a CallbackObjectBase r?smaug — Details

Instead, get required information directly.

Matthew Gaudet (he/him) [:mgaudet]

Assignee

Comment 5

•

1 year ago

Attached file Bug 1983154 - Add javascript.options.use_js_microtask_queue r?arai — Details

Matthew Gaudet (he/him) [:mgaudet]

Assignee

Comment 6

•

1 year ago

Attached file Bug 1983154 - Create MaybeGetPromiseAllocationSite r?arai — Details

This is a more flexibile version

Matthew Gaudet (he/him) [:mgaudet]

Assignee

Comment 7

•

1 year ago

Attached file Bug 1983154 - Shell support for a JS MicroTaskQueue. r?arai — Details

Matthew Gaudet (he/him) [:mgaudet]

Assignee

Comment 8

•

1 year ago

Attached file Bug 1983154 - Gecko Support for JS Micro Task Queue r?arai!,smaug! — Details

Matthew Gaudet (he/him) [:mgaudet]

Assignee

Comment 9

•

1 year ago

Patches just uploaded are WIP still, but worth sharing as the cover the general thrust of what we're looking at.

Tooru Fujisawa [:arai]

Updated

•

1 year ago

Updated

•

1 year ago

Assignee: nobody → mgaudet

Severity: -- → N/A

Priority: -- → P1

Phabricator Automation

Updated

•

11 months ago

Attachment #9507213 - Attachment is obsolete: true

Matthew Gaudet (he/him) [:mgaudet]

Assignee

Comment 10

•

11 months ago

Attached file Bug 1983154 - Make Vector a parameter to ds/fifo.h r?jonco — Details

Matthew Gaudet (he/him) [:mgaudet]

Assignee

Comment 11

•

11 months ago

Attached file Bug 1983154 - Support JS::Value in TraceableFifo r?jonco — Details

Phabricator Automation

Updated

•

11 months ago

Attachment #9507214 - Attachment description: WIP: Bug 1983154 - Move ReactionRecord slots inside of class → Bug 1983154 - Move ReactionRecord slots inside of class r?arai

Phabricator Automation

Updated

•

11 months ago

Attachment #9507215 - Attachment description: WIP: Bug 1983154 - Split CallSetup out of CallbackObjectBase → Bug 1983154 - Split CallSetup out of CallbackObjectBase r?smaug

Phabricator Automation

Updated

•

11 months ago

Attachment #9507216 - Attachment description: WIP: Bug 1983154 - Add CallSetup that doesn't take a CallbackObjectBase → Bug 1983154 - Add CallSetup that doesn't take a CallbackObjectBase r?smaug

Phabricator Automation

Updated

•

11 months ago

Attachment #9507217 - Attachment description: WIP: Bug 1983154 - Add javascript.options.use_js_microtask_queue → Bug 1983154 - Add javascript.options.use_js_microtask_queue r?arai

Phabricator Automation

Updated

•

11 months ago

Attachment #9507218 - Attachment description: WIP: Bug 1983154 - Create MaybeGetPromiseAllocationSite → Bug 1983154 - Create MaybeGetPromiseAllocationSite r?arai

Phabricator Automation

Updated

•

11 months ago

Attachment #9507219 - Attachment description: WIP: Bug 1983154 - Shell support for a JS MicroTaskQueue. → Bug 1983154 - Shell support for a JS MicroTaskQueue. r?arai

Phabricator Automation

Updated

•

11 months ago

Attachment #9507220 - Attachment description: WIP: Bug 1983154 - Gecko Support for JS Micro Task Queue → Bug 1983154 - Gecko Support for JS Micro Task Queue r?arai!,smaug!

Matthew Gaudet (he/him) [:mgaudet]

Assignee

Comment 12

•

11 months ago

Attached file Bug 1983154 - Select debugger queue where required r?arai!,smaug! — Details

I don't love having a virtual dispatch on this hot path, but this is required for matching
existing semantics, as revealed by dom/workers/test/test_WorkerDebugger_promise.xhtml

Longer term I'd love to do something else.

Matthew Gaudet (he/him) [:mgaudet]

Assignee

Comment 13

•

11 months ago

Attached file Bug 1983154 - Track the correct execution global more carefully. r?arai! (obsolete) — Details

Matthew Gaudet (he/him) [:mgaudet]

Assignee

Comment 14

•

11 months ago

Ok. Patches are up for review now. The goal right now is to land this disabled, and then once it's landed disabled we can collaboratively get the feature ready for shipping.

There's still a few known hangs from my most recent enabled run on try, but I don't think that needs to block review.

Matthew Gaudet (he/him) [:mgaudet]

Assignee

Updated

•

11 months ago

Blocks: 1987692

Phabricator Automation

Updated

•

11 months ago

Attachment #9512104 - Attachment is obsolete: true

Matthew Gaudet (he/him) [:mgaudet]

Assignee

Comment 15

•

11 months ago

Attached file Bug 1983154 - Refactor CallSetup r?smaug — Details

This refactoring splits the CallSetup constructor into some recyclable pieces.

Pulsebot

Comment 16

•

11 months ago

Pulsebot

Comment 17

•

11 months ago

Pushed by imoraru@mozilla.com: https://github.com/mozilla-firefox/firefox/commit/80d03cf16647 https://hg.mozilla.org/integration/autoland/rev/a5c47db18247 Revert "Bug 1983154 Bug 1977466 - Select debugger queue where required r=arai,smaug,dom-worker-reviewers,asuth" for causing linux build bustages on Fifo.h.

Iulian Moraru

Comment 18

•

11 months ago

•

Edited

Revert for causing linux build bustages on Fifo.h.

[task 2025-09-26T20:27:35.173+00:00] 20:27:35     INFO -  gmake[4]: Entering directory '/builds/worker/workspace/obj-build/js/src/intgemm'
[task 2025-09-26T20:27:35.175+00:00] 20:27:35     INFO -  /builds/worker/fetches/sccache/sccache /builds/worker/fetches/clang/bin/clang++ --sysroot /builds/worker/fetches/sysroot-x86_64-linux-gnu -o IntegerGemmIntrinsic.o -c  -I/builds/worker/workspace/obj-build/dist/stl_wrappers -I/builds/worker/workspace/obj-build/dist/system_wrappers -include /builds/worker/checkouts/gecko/config/gcc_hidden.h -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -D_GLIBCXX_ASSERTIONS -fstack-protector-strong -fstack-clash-protection -ftrivial-auto-var-init=pattern -fstrict-flex-arrays=1 -DDEBUG=1 -DWASM_SUPPORTS_HUGE_MEMORY -DJS_CACHEIR_SPEW -DJS_STRUCTURED_SPEW -DJS_HAS_CTYPES -DFFI_BUILDING -DUSE_SSE2 -DUSE_SSSE3 -DUSE_AVX2 -DUSE_AVXVNNI -DUSE_AVX512BW -DUSE_AVX512VNNI -DEXPORT_JS_API -DMOZ_HAS_MOZGLUE -DMOZ_SUPPORT_LEAKCHECKING -I/builds/worker/checkouts/gecko/js/src/intgemm -I/builds/worker/workspace/obj-build/js/src/intgemm -I/builds/worker/workspace/obj-build/js/src -I/builds/worker/checkouts/gecko/js/src -I/builds/worker/checkouts/gecko/third_party/gemmology -I/builds/worker/checkouts/gecko/third_party/xsimd/include -I/builds/worker/workspace/obj-build/dist/include -I/builds/worker/workspace/obj-build/dist/include/nspr -I/builds/worker/workspace/obj-build/dist/include/nss -DMOZILLA_CLIENT -include /builds/worker/workspace/obj-build/mozilla-config.h -D_GLIBCXX_USE_CXX11_ABI=0 -fno-rtti -pthread -fno-sized-deallocation -fno-aligned-new -ffunction-sections -fdata-sections -fno-math-errno -fno-exceptions -fPIC -fcrash-diagnostics-dir=/builds/worker/artifacts -gdwarf-4 -O3 -fno-omit-frame-pointer -funwind-tables -Wall -Wbitfield-enum-conversion -Wempty-body -Wformat-type-confusion -Wignored-qualifiers -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtautological-constant-in-range-compare -Wtype-limits -Wno-error=tautological-type-limit-compare -Wunreachable-code -Wunreachable-code-return -Wunused-but-set-parameter -Wno-invalid-offsetof -Wclass-varargs -Wempty-init-stmt -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wno-range-loop-analysis -Wenum-compare-conditional -Wenum-float-conversion -Wno-deprecated-anon-enum-enum-conversion -Wno-deprecated-enum-enum-conversion -Wno-deprecated-this-capture -Wcomma -Wimplicit-fallthrough -Wstring-conversion -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=free-nonheap-object -Wno-error=atomic-alignment -Wno-error=deprecated-builtins -Wformat -Wformat-security -Wno-psabi -Wthread-safety -Wno-error=builtin-macro-redefined -Wno-unknown-warning-option -Werror=format -fno-strict-aliasing -ffp-contract=off  -MD -MP -MF .deps/IntegerGemmIntrinsic.o.pp   /builds/worker/checkouts/gecko/js/src/intgemm/IntegerGemmIntrinsic.cpp
[task 2025-09-26T20:27:35.175+00:00] 20:27:35     INFO -  In file included from /builds/worker/checkouts/gecko/js/src/intgemm/IntegerGemmIntrinsic.cpp:22:
[task 2025-09-26T20:27:35.176+00:00] 20:27:35     INFO -  In file included from /builds/worker/checkouts/gecko/js/src/vm/JSContext.h:19:
[task 2025-09-26T20:27:35.176+00:00] 20:27:35     INFO -  In file included from /builds/worker/checkouts/gecko/js/src/ds/TraceableFifo.h:10:
[task 2025-09-26T20:27:35.176+00:00] 20:27:35    ERROR -  /builds/worker/checkouts/gecko/js/src/ds/Fifo.h:30:65: error: template template argument has different template parameters than its corresponding template template parameter
[task 2025-09-26T20:27:35.176+00:00] 20:27:35     INFO -     30 |           template <typename, size_t, class> class VectorType = Vector>
[task 2025-09-26T20:27:35.176+00:00] 20:27:35     INFO -        |                                                                 ^
[task 2025-09-26T20:27:35.177+00:00] 20:27:35     INFO -  /builds/worker/checkouts/gecko/js/src/vm/OffThreadPromiseRuntimeState.h:220:63: note: while checking a default template argument used here
[task 2025-09-26T20:27:35.177+00:00] 20:27:35     INFO -    220 |     Fifo<js::UniquePtr<JS::Dispatchable>, 0, SystemAllocPolicy>;
[task 2025-09-26T20:27:35.177+00:00] 20:27:35     INFO -        |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
[task 2025-09-26T20:27:35.177+00:00] 20:27:35     INFO -  /builds/worker/workspace/obj-build/dist/include/js/Vector.h:30:1: note: too many template parameters in template template argument
[task 2025-09-26T20:27:35.177+00:00] 20:27:35     INFO -     30 | template <typename T, size_t MinInlineCapacity = 0,
[task 2025-09-26T20:27:35.177+00:00] 20:27:35     INFO -        | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[task 2025-09-26T20:27:35.178+00:00] 20:27:35     INFO -     31 |           class AllocPolicy = TempAllocPolicy,
[task 2025-09-26T20:27:35.178+00:00] 20:27:35     INFO -        |           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[task 2025-09-26T20:27:35.178+00:00] 20:27:35     INFO -     32 |           // Don't use this with JS::Value!  Use JS::RootedValueVector instead.
[task 2025-09-26T20:27:35.178+00:00] 20:27:35     INFO -        |           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[task 2025-09-26T20:27:35.178+00:00] 20:27:35     INFO -     33 |           typename = std::enable_if_t<!detail::TypeIsGCThing<T>::value>>
[task 2025-09-26T20:27:35.178+00:00] 20:27:35     INFO -        |           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[task 2025-09-26T20:27:35.179+00:00] 20:27:35     INFO -  /builds/worker/checkouts/gecko/js/src/ds/Fifo.h:30:11: note: previous template template parameter is here
[task 2025-09-26T20:27:35.179+00:00] 20:27:35     INFO -     30 |           template <typename, size_t, class> class VectorType = Vector>
[task 2025-09-26T20:27:35.179+00:00] 20:27:35     INFO -        |           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[task 2025-09-26T20:27:35.179+00:00] 20:27:35     INFO -  In file included from /builds/worker/checkouts/gecko/js/src/intgemm/IntegerGemmIntrinsic.cpp:22:
[task 2025-09-26T20:27:35.179+00:00] 20:27:35     INFO -  In file included from /builds/worker/checkouts/gecko/js/src/vm/JSContext.h:41:
[task 2025-09-26T20:27:35.180+00:00] 20:27:35     INFO -  In file included from /builds/worker/checkouts/gecko/js/src/vm/Runtime.h:57:
[task 2025-09-26T20:27:35.180+00:00] 20:27:35    ERROR -  /builds/worker/checkouts/gecko/js/src/vm/OffThreadPromiseRuntimeState.h:292:24: error: use of undeclared identifier 'DispatchableFifo'
[task 2025-09-26T20:27:35.180+00:00] 20:27:35     INFO -    292 |   HelperThreadLockData<DispatchableFifo> failed_;
[task 2025-09-26T20:27:35.180+00:00] 20:27:35     INFO -        |                        ^
[task 2025-09-26T20:27:35.180+00:00] 20:27:35    ERROR -  /builds/worker/checkouts/gecko/js/src/vm/OffThreadPromiseRuntimeState.h:305:24: error: use of undeclared identifier 'DispatchableFifo'
[task 2025-09-26T20:27:35.180+00:00] 20:27:35     INFO -    305 |   HelperThreadLockData<DispatchableFifo> internalDispatchQueue_;
[task 2025-09-26T20:27:35.181+00:00] 20:27:35     INFO -        |                        ^
[task 2025-09-26T20:27:35.181+00:00] 20:27:35    ERROR -  /builds/worker/checkouts/gecko/js/src/vm/OffThreadPromiseRuntimeState.h:313:3: error: unknown type name 'DispatchableFifo'
[task 2025-09-26T20:27:35.181+00:00] 20:27:35     INFO -    313 |   DispatchableFifo& failed() { return failed_.ref(); }
[task 2025-09-26T20:27:35.181+00:00] 20:27:35     INFO -        |   ^
[task 2025-09-26T20:27:35.181+00:00] 20:27:35    ERROR -  /builds/worker/checkouts/gecko/js/src/vm/OffThreadPromiseRuntimeState.h:316:3: error: unknown type name 'DispatchableFifo'
[task 2025-09-26T20:27:35.181+00:00] 20:27:35     INFO -    316 |   DispatchableFifo& internalDispatchQueue() {
[task 2025-09-26T20:27:35.182+00:00] 20:27:35     INFO -        |   ^
[task 2025-09-26T20:27:35.182+00:00] 20:27:35     INFO -  5 errors generated.
[task 2025-09-26T20:27:35.182+00:00] 20:27:35    ERROR -  gmake[4]: *** [/builds/worker/checkouts/gecko/config/rules.mk:669: IntegerGemmIntrinsic.o] Error 1
[task 2025-09-26T20:27:35.182+00:00] 20:27:35     INFO -  gmake[4]: Leaving directory '/builds/worker/workspace/obj-build/js/src/intgemm'
<...>

Flags: needinfo?(mgaudet)

Pulsebot

Comment 19

•

11 months ago

Pulsebot

Comment 20

•

11 months ago

Pushed by amarc@mozilla.com: https://github.com/mozilla-firefox/firefox/commit/fb8e963ad98f https://hg.mozilla.org/integration/autoland/rev/24389fca49b5 Revert (Bug 1983154, Bug 1977466) for causing hazard bustages @ CallbackObject.cpp

amarc

Comment 21

•

11 months ago

Backed out for causing hazard bustages

Pulsebot

Comment 22

•

11 months ago

Pulsebot

Comment 23

•

11 months ago

Pushed by abutkovits@mozilla.com: https://github.com/mozilla-firefox/firefox/commit/4258ca077fef https://hg.mozilla.org/integration/autoland/rev/24347d6b356b Revert "Bug 1983154, Bug 1977466 - Select debugger queue where required r=arai,smaug,dom-worker-reviewers,asuth" for causing Hazard bustages.

Atila Butkovits

Comment 24

•

11 months ago

Backed out for causing Hazard bustages.

Backout link: https://hg-edge.mozilla.org/integration/autoland/rev/24347d6b356b78c7167efdd14ed5ac8d22f08735

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&searchStr=hazard&revision=86a95a0062ed3e2261e648ff889d019bd8fe8cc6&selectedTaskRun=bRT9mfu1QOO0qdU3ib8PXg.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=529254653&repo=autoland&task=bRT9mfu1QOO0qdU3ib8PXg.0&lineNumber=157178

Pulsebot

Comment 25

•

11 months ago

Cristian Tuns

Comment 26

•

11 months ago

bugherder

Status: NEW → RESOLVED

Closed: 11 months ago

status-firefox145: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → 145 Branch

Matthew Gaudet (he/him) [:mgaudet]

Assignee

Updated

•

10 months ago

Flags: needinfo?(mgaudet)

Mihai Boldan, Desktop QA [:mboldan]

Updated

•

10 months ago

QA Whiteboard: [qa-triage-done-c146/b145]

Ryan VanderMeulen [:RyanVM]

Updated

•

10 months ago

Regressions: 1995409

Mayank Bansal

Updated

•

10 months ago

Depends on: 1997702

Matthew Gaudet (he/him) [:mgaudet]

Assignee

Updated

•

9 months ago

Depends on: 2002852

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