1934108 - Cleanup existing JIT-code preservation code

Status: RESOLVED FIXED

(Core :: JavaScript: GC, task, P2)

Description

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

This bug is for tracking some non-functional changes to how JIT code is managed by GC (renaming, documentation comments etc)

Comment 1

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

Attachment: Bug 1934108 - Document why RealmCreationOptions::setPreserveJitCodeExists r?jonco

Comment 2

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

Attachment: Bug 1934108 - Just check the GC options rather than the cleanUpEverything boolean r?jonco

There -is- a semantic change here. In GCRuntime::maybeCallGCCallback we save and restore
the gcOptions, lest we re-enter GC. This means there are a couple of possible changes
in behaviour from this change (and I'm honestly not sure which happens here, or
if ultimately this matters at all):

1. The first option is that as a result of this, a nested GC triggered by a GC
   callback which would have previouly done a 'cleanupEverything' GC no longer
   does, as the nested GC may have a different GCOptions.

2. The second option is that nested GC could have accidentally been clearing
   the cleanupEverything field, and now it no longer does. As a result we do
   -more- cleanupEverything GC now than we did before.

Comment 3

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

Attachment: Bug 1934108 - Document why RealmCreationOptions::setPreserveJitCodeExists r?jonco

Comment 4

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

Attachment: Bug 1934108 - Just check the GC options rather than the cleanUpEverything boolean r?jonco

There -is- a semantic change here. In GCRuntime::maybeCallGCCallback we save and restore
the gcOptions, lest we re-enter GC. This means there are a couple of possible changes
in behaviour from this change (and I'm honestly not sure which happens here, or
if ultimately this matters at all):

1. The first option is that as a result of this, a nested GC triggered by a GC
   callback which would have previouly done a 'cleanupEverything' GC no longer
   does, as the nested GC may have a different GCOptions.

2. The second option is that nested GC could have accidentally been clearing
   the cleanupEverything field, and now it no longer does. As a result we do
   -more- cleanupEverything GC now than we did before.

Comment 5

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

Attachment: Bug 1934108 - Remove helper function and explain more directly the heuristics in shouldPreserveJITCode r?jonco

Comment 6

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

Attachment: Bug 1934108 - Document current code preservation reasons r?jonco

Comment 7

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

Attachment: Bug 1934108 - Comment on CancelOffThreadCompile r?jonco

Comment 8

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

Attachment: Bug 1934108 - Don't bother calling into discardJitCode if we're not preserving any code r?jonco

Here we should just straight to forceDiscardJitCode.

Comment 9

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

Attachment: Bug 1934108 - Rename discardJitCode to maybeDiscardJitCode r?jonco

Clarify this functions name

Comment 10

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

Attachment: Bug 1934108 - Rename DiscardOptions to JitDiscardOptions to make it more clear r?jonco

Comment 11

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

Attachment: Bug 1934108 - Remove JitDiscardOptions from maybeDiscardJitCode r?jonco

No caller ever provides any non-default.

Comment 12

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

Attachment: Bug 1934108 - Rename haveDiscardJitCodeThisSlice r?jonco

Comment 13

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

Attachment: Bug 1934108 - Rename discardJITCodeForGC r?jonco

Comment 14

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

Attachment: Bug 1934108 - Detail a comment in forceDiscardJitCode r?jonco

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/712aad5a0b58
Document why RealmCreationOptions::setPreserveJitCodeExists r=jonco
https://hg.mozilla.org/integration/autoland/rev/9ff939988c61
Just check the GC options rather than the cleanUpEverything boolean r=jonco
https://hg.mozilla.org/integration/autoland/rev/de793cbd8de5
Remove helper function and explain more directly the heuristics in shouldPreserveJITCode r=jonco
https://hg.mozilla.org/integration/autoland/rev/e194475daa2e
Document current code preservation reasons r=jonco
https://hg.mozilla.org/integration/autoland/rev/7dc64ac5009a
Don't bother calling into discardJitCode if we're not preserving any code r=jonco
https://hg.mozilla.org/integration/autoland/rev/6ba7346640ee
Rename discardJitCode to maybeDiscardJitCode r=jonco
https://hg.mozilla.org/integration/autoland/rev/8405785b43b7
Rename DiscardOptions to JitDiscardOptions to make it more clear r=jonco
https://hg.mozilla.org/integration/autoland/rev/9c56922c616b
Remove JitDiscardOptions from maybeDiscardJitCode r=jonco
https://hg.mozilla.org/integration/autoland/rev/a773e246306b
Rename discardJITCodeForGC r=jonco
https://hg.mozilla.org/integration/autoland/rev/efb91e901151
Detail a comment in forceDiscardJitCode r=jonco
https://hg.mozilla.org/integration/autoland/rev/44cadd43898a
apply code formatting via Lando

Comment 16

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/712aad5a0b58
https://hg.mozilla.org/mozilla-central/rev/9ff939988c61
https://hg.mozilla.org/mozilla-central/rev/de793cbd8de5
https://hg.mozilla.org/mozilla-central/rev/e194475daa2e
https://hg.mozilla.org/mozilla-central/rev/7dc64ac5009a
https://hg.mozilla.org/mozilla-central/rev/6ba7346640ee
https://hg.mozilla.org/mozilla-central/rev/8405785b43b7
https://hg.mozilla.org/mozilla-central/rev/9c56922c616b
https://hg.mozilla.org/mozilla-central/rev/a773e246306b
https://hg.mozilla.org/mozilla-central/rev/efb91e901151
https://hg.mozilla.org/mozilla-central/rev/44cadd43898a