2009858 - There is still noticiable rooting cost in doxbee-promise

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P3)

Description

[Matthew Gaudet (he/him) [:mgaudet]]

Visible in this recent profile: https://share.firefox.dev/3Nj3nDx

We can get rid of some of this by using RootedTuple/RootedField.

Comment 1

[Matthew Gaudet (he/him) [:mgaudet]]

Reworking that profile you can spot a bunch more opportunities, so I'm resummarying this bug and taking it.

I've got a stack that I locally measure at a 1.5-2% improvement just from adjusting rooting.

Comment 2

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 2009858 - Eliminate excess rooting overhead in EnqueuePromiseReactionJob r?arai

Comment 3

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 2009858 - Eliminate excess rooting overhead in js::ResolvePromiseInternal r?arai

Comment 4

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 2009858 - Eliminate excess rooting overhead in js::ReactToUnwrappedPromise r?arai

Comment 5

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 2009858 - Eliminate excess rooting overhead in InternalAwait r?arai

Comment 6

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 2009858 - Support conversion to superclass handle type for RootedField r?jonco

Used in the next patch in the stack to handle a conversion from

RootedField<PromiseObject*> to Handle<JSObject*>

Comment 7

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 2009858 - Eliminate excess rooting overhead in js::AsyncFromSyncIteratorMethod r?arai

Comment 8

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 2009858 - Eliminate excess rooting overhead in PromiseReactionJob r?arai

Comment 9

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 2009858 - Eliminate excess rooting overhead in PromiseResolveThenableJob r?arai

Comment 10

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 2009858 - Eliminate excess rooting overhead in EnqueuePromiseResolveThenableJob r?arai

Comment 11

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 2009858 - Eliminate excess rooting overhead in js::OriginalPromiseThen r?arai

Comment 12

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 2009858 - Eliminate excess rooting overhead in PromiseObject::forEachReachReactionRecord r?arai

Comment 13

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 2009858 - Eliminate excess rooting overhead in FulfillMaybeWrappedPromise r?arai

Comment 14

[Matthew Gaudet (he/him) [:mgaudet]]

Attachment: Bug 2009858 - Always default initialize a RootedFieldValue r?jonco

Otherwise translation to RootedField can be a refactoring hazard. Consider

RootedTuple<JSObject*> roots(cx); 
for (x : container) {
	RootedField<JSObject*, 0> field(roots); 
	if (x.condition) {
		field = x.obj;
	}

	if (field && x.condition_two) {
		do_thing(field);
	}
}

In this loop, without re-initializing field, we can leak obj from a previous
iteration to a new one.

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/34d3dad9c98a
https://hg.mozilla.org/integration/autoland/rev/f2b4c61c9966
Eliminate excess rooting overhead in EnqueuePromiseReactionJob r=arai
https://github.com/mozilla-firefox/firefox/commit/087b3564324d
https://hg.mozilla.org/integration/autoland/rev/21bd49d8d0f6
Eliminate excess rooting overhead in js::ResolvePromiseInternal r=arai
https://github.com/mozilla-firefox/firefox/commit/23234dc94ddb
https://hg.mozilla.org/integration/autoland/rev/6111673a2fa1
Eliminate excess rooting overhead in js::ReactToUnwrappedPromise r=arai
https://github.com/mozilla-firefox/firefox/commit/2f12fa226f88
https://hg.mozilla.org/integration/autoland/rev/1a3959df8ee5
Eliminate excess rooting overhead in InternalAwait r=arai
https://github.com/mozilla-firefox/firefox/commit/37ded86b0f9c
https://hg.mozilla.org/integration/autoland/rev/2bd3be20e763
Support conversion to superclass handle type for RootedField r=jonco
https://github.com/mozilla-firefox/firefox/commit/17d5c7ccf9b7
https://hg.mozilla.org/integration/autoland/rev/6a5705563ff8
Eliminate excess rooting overhead in  js::AsyncFromSyncIteratorMethod r=arai
https://github.com/mozilla-firefox/firefox/commit/35eb641ef895
https://hg.mozilla.org/integration/autoland/rev/bba9e03de600
Eliminate excess rooting overhead in PromiseReactionJob r=arai
https://github.com/mozilla-firefox/firefox/commit/c852a41b8423
https://hg.mozilla.org/integration/autoland/rev/2fb554957418
Eliminate excess rooting overhead in PromiseResolveThenableJob r=arai
https://github.com/mozilla-firefox/firefox/commit/926b80d36b7b
https://hg.mozilla.org/integration/autoland/rev/add9353df378
Eliminate excess rooting overhead in EnqueuePromiseResolveThenableJob r=arai
https://github.com/mozilla-firefox/firefox/commit/a624ab4f69e9
https://hg.mozilla.org/integration/autoland/rev/c63165bb96b3
Eliminate excess rooting overhead in js::OriginalPromiseThen r=arai
https://github.com/mozilla-firefox/firefox/commit/7e18f6bacaa0
https://hg.mozilla.org/integration/autoland/rev/b56a322991f0
Always default initialize a RootedFieldValue r=jonco
https://github.com/mozilla-firefox/firefox/commit/53f8c7d9b64c
https://hg.mozilla.org/integration/autoland/rev/be31b687694c
Eliminate excess rooting overhead in FulfillMaybeWrappedPromise r=arai
https://github.com/mozilla-firefox/firefox/commit/b371451c3a65
https://hg.mozilla.org/integration/autoland/rev/2e741a895ef5
Eliminate excess rooting overhead in PromiseObject::forEachReachReactionRecord r=arai

Comment 16

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/f2b4c61c9966
https://hg.mozilla.org/mozilla-central/rev/21bd49d8d0f6
https://hg.mozilla.org/mozilla-central/rev/6111673a2fa1
https://hg.mozilla.org/mozilla-central/rev/1a3959df8ee5
https://hg.mozilla.org/mozilla-central/rev/2bd3be20e763
https://hg.mozilla.org/mozilla-central/rev/6a5705563ff8
https://hg.mozilla.org/mozilla-central/rev/bba9e03de600
https://hg.mozilla.org/mozilla-central/rev/2fb554957418
https://hg.mozilla.org/mozilla-central/rev/add9353df378
https://hg.mozilla.org/mozilla-central/rev/c63165bb96b3
https://hg.mozilla.org/mozilla-central/rev/b56a322991f0
https://hg.mozilla.org/mozilla-central/rev/be31b687694c
https://hg.mozilla.org/mozilla-central/rev/2e741a895ef5