1836700 - Eliminate busy count tracking from WorkerRunnables

Status: RESOLVED FIXED

(Core :: DOM: Workers, enhancement)

Description

[Andrew Sutherland [:asuth] (he/him)]

Eliminate busy count tracking in favor of just letting WorkerRefs, the existence of IPC actors, and (newly proposed) the presence of events in our main-thread ThrottledEventQueues/worker debugee queues keep a worker alive. This enables a simplification of the WorkerRunnable implementation. This is a spin-off follow-up from bug 1800659 which going to originally address this (but which quite reasonably was de-scoped).

One particular benefit is there is no need for runnables sent to the main thread to have a reference to the WorkerPrivate (to update the busy count), however it's likely that we would want to approximate the original design behavior by having the GC traversal logic in WorkerPrivate::Traverse that Worker::Traverse calls into:

• Check if there are any runnables in our mMainThreadDebuggeeEventTarget main thread-targeted ThrottledEventQueue. This is a new proposal with the filing of this bug, but an easy thing for us to check that could potentially allow us to continue to GC in more cases.
• We also would like to be aware of runnables resulting from Worker.postMessage; most practically having any runnables in the queue would be sufficient but would err on the side of GCing less.
• Check if there are StrongWorkerRefs. (IPCWorkerRefs would continue to not prevent GC, as would also be the case for WeakWorkerRefs.)

Comment 1

[Eden Chuang[:edenchuang]]

I have some local patches for this, so I take this bug.

Comment 2

[Eden Chuang[:edenchuang]]

Attachment: Bug 1836700 - Remove BusyCount of WorkerPrivate. r=#dom-worker-reviewers

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ff2400205932
Remove BusyCount of WorkerPrivate. r=asuth,ipc-reviewers,mccr8

Comment 4

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/ff2400205932

Comment 5

[Nicolas Chevobbe [:nchevobbe]]

== Change summary for alert #40233 (as of Tue, 14 Nov 2023 14:05:28 GMT) ==

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
21%	damp source-map-loader.getOriginalLocation.DAMP	linux1804-64-shippable-qr	e10s fission stylo webrender-sw	133.77 -> 105.90
21%	damp source-map-loader.getOriginalLocation.DAMP	linux1804-64-shippable-qr	e10s fission stylo webrender	136.95 -> 108.79
16%	damp source-map-loader.getOriginalLocation.DAMP	windows10-64-shippable-qr	e10s fission stylo webrender	183.72 -> 154.83
15%	damp source-map-loader.getOriginalLocation.DAMP	windows10-64-shippable-qr	e10s fission stylo webrender-sw	187.28 -> 159.12
13%	damp source-map-loader.getOriginalLocation.DAMP	macosx1015-64-shippable-qr	e10s fission stylo webrender	158.75 -> 138.26
...	...	...	...	...
2%	damp custom.jsdebugger.project-search.DAMP	windows10-64-shippable-qr	e10s fission stylo webrender	2,153.88 -> 2,109.49

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=40233