1723715 - Move data stored in reserved slots on the global to a C++ class

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P3)

Description

[Jan de Mooij [:jandem]]

Global objects have about 200 reserved slots that store the builtin prototypes and constructors, and various other bits of information. This has gotten out of hand and has the following issues:

• Each time we grow the slots when adding properties to the global object we have to copy over these slots.
• These values are stored as JS::Value so we lose type safety.
• Some of the slots store simple booleans, pretty wasteful.
• JSClass currently allows at most 256 reserved slots and we're not that far away from this.

The plan is to move this to a C++ class that we can attach to the global. This class is similar to Realm, but moving these slots to the realm is complicated due to GC lifetime issues.

One wrinkle is that we can't easily add a finalizer for this because the global's JSClass is defined by the embedder, but we can likely free this via the realm.

Comment 1

[Jan de Mooij [:jandem]]

Another one:

• Adding or removing a slot requires updating JSCLASS_GLOBAL_SLOT_COUNT.

Comment 2

[Jan de Mooij [:jandem]]

Attachment: Bug 1723715 part 1 - Add GlobalObjectData and use it for emptyGlobalScope. r?jonco!

Comment 3

[Jan de Mooij [:jandem]]

Attachment: Bug 1723715 part 2 - Remove the RUNTIME_CODEGEN_ENABLED slot. r?jonco!

Originally this slot was used to cache the return value of the callback. However
that was changed at some point, now the slot is only set to TrueValue when there
is no callback installed. It's now better and simpler to remove the slot.

Depends on D121981

Comment 4

[Jan de Mooij [:jandem]]

Attachment: Bug 1723715 part 3 - Move RegExpStaticsObject to GlobalObjectData. r?jonco!

RegExpStaticsObject just stores the RegExpStatics. It might be possible to store
the RegExpStatics directly in the GlobalObjectData in a follow-up bug.

Depends on D121982

Comment 5

[Jan de Mooij [:jandem]]

Attachment: Bug 1723715 part 4 - Move intrinsics holder to GlobalObjectData. r?jonco!

Depends on D121983

Comment 6

[Jan de Mooij [:jandem]]

Attachment: Bug 1723715 part 5 - Move ForOfPICChain to GlobalObjectData. r?jonco!

Depends on D121984

Comment 7

[Jan de Mooij [:jandem]]

Attachment: Bug 1723715 part 6 - Move globalThisResolved to GlobalObjectData. r?jonco!

Depends on D121985

Comment 8

[Jan de Mooij [:jandem]]

Attachment: Bug 1723715 part 7 - Move SourceURLsHolder to GlobalObjectData. r?jonco!

Depends on D121986

Comment 9

[Jan de Mooij [:jandem]]

Attachment: Bug 1723715 part 8 - Move realm key object to GlobalObjectData. r?jonco!

Depends on D121987

Comment 10

[Jan de Mooij [:jandem]]

Attachment: Bug 1723715 part 9 - Move array shape to GlobalObjectData. r?jonco!

Depends on D121988

Comment 11

[Jan de Mooij [:jandem]]

Attachment: Bug 1723715 part 10 - Move ThrowTypeError to GlobalObjectData. r?jonco!

Depends on D121989

Comment 12

[Jan de Mooij [:jandem]]

Attachment: Bug 1723715 part 11 - Move eval function to GlobalObjectData. r?jonco!

Depends on D121990

Comment 13

[Jan de Mooij [:jandem]]

Attachment: Bug 1723715 part 12 - Move some builtin prototypes to GlobalObjectData. r?jonco!

Depends on D121991

Comment 14

[Jan de Mooij [:jandem]]

Attachment: Bug 1723715 part 13 - Move builtin constructors to GlobalObjectData. r?jonco!

This replaces the JS_OFF_THREAD_CONSTRUCTOR MagicValue for off-thread constructors
with the same placeholder object we use for the prototype. These constructors
aren't used by off-thread parsing and handling this another way requires a lot
of complexity. With Stencil work the off-thread global will hopefully be removed
eventually.

Comment 15

[Jan de Mooij [:jandem]]

Attachment: Bug 1723715 part 14 - Clean up slot related code a bit. r?jonco!

Comment 16

[Jan de Mooij [:jandem]]

Attachment: Bug 1723715 part 15 - Move global lexical environment from Realm to GlobalObjectData. r?jonco!

This adds a few dereferences to the Baseline Interpreter, but is simpler and should
be faster for C++ accesses. It also simplifies/unblocks the next patch.

Comment 17

[Jan de Mooij [:jandem]]

Attachment: Bug 1723715 part 16 - Move WindowProxy to GlobalObjectData. r?jonco!

This is done last because it requires reordering the initialization code, and we
can finally do this now.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/12e1339f1e0c
part 1 - Add GlobalObjectData and use it for emptyGlobalScope. r=jonco
https://hg.mozilla.org/integration/autoland/rev/d163b4821d80
part 2 - Remove the RUNTIME_CODEGEN_ENABLED slot. r=jonco
https://hg.mozilla.org/integration/autoland/rev/1bcb3ef4165f
part 3 - Move RegExpStaticsObject to GlobalObjectData. r=jonco
https://hg.mozilla.org/integration/autoland/rev/ed50c6a66971
part 4 - Move intrinsics holder to GlobalObjectData. r=jonco
https://hg.mozilla.org/integration/autoland/rev/73f320ef0d12
part 5 - Move ForOfPICChain to GlobalObjectData. r=jonco
https://hg.mozilla.org/integration/autoland/rev/3cb4aa71f8e8
part 6 - Move globalThisResolved to GlobalObjectData. r=jonco
https://hg.mozilla.org/integration/autoland/rev/8556d887ee8b
part 7 - Move SourceURLsHolder to GlobalObjectData. r=jonco
https://hg.mozilla.org/integration/autoland/rev/348de7b61aa0
part 8 - Move realm key object to GlobalObjectData. r=jonco
https://hg.mozilla.org/integration/autoland/rev/371c84d4007e
part 9 - Move array shape to GlobalObjectData. r=jonco
https://hg.mozilla.org/integration/autoland/rev/1fbf0da32180
part 10 - Move ThrowTypeError to GlobalObjectData. r=jonco
https://hg.mozilla.org/integration/autoland/rev/71d833b689b6
part 11 - Move eval function to GlobalObjectData. r=jonco
https://hg.mozilla.org/integration/autoland/rev/f73777bc2877
part 12 - Move some builtin prototypes to GlobalObjectData. r=jonco
https://hg.mozilla.org/integration/autoland/rev/eeb83afc429e
part 13 - Move builtin constructors to GlobalObjectData. r=jonco
https://hg.mozilla.org/integration/autoland/rev/61dd98c602d8
part 14 - Clean up slot related code a bit. r=jonco
https://hg.mozilla.org/integration/autoland/rev/c820d2e6087e
part 15 - Move global lexical environment from Realm to GlobalObjectData. r=jonco
https://hg.mozilla.org/integration/autoland/rev/5fbcd90b2e8f
part 16 - Move WindowProxy to GlobalObjectData. r=jonco

Comment 19

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/12e1339f1e0c
https://hg.mozilla.org/mozilla-central/rev/d163b4821d80
https://hg.mozilla.org/mozilla-central/rev/1bcb3ef4165f
https://hg.mozilla.org/mozilla-central/rev/ed50c6a66971
https://hg.mozilla.org/mozilla-central/rev/73f320ef0d12
https://hg.mozilla.org/mozilla-central/rev/3cb4aa71f8e8
https://hg.mozilla.org/mozilla-central/rev/8556d887ee8b
https://hg.mozilla.org/mozilla-central/rev/348de7b61aa0
https://hg.mozilla.org/mozilla-central/rev/371c84d4007e
https://hg.mozilla.org/mozilla-central/rev/1fbf0da32180
https://hg.mozilla.org/mozilla-central/rev/71d833b689b6
https://hg.mozilla.org/mozilla-central/rev/f73777bc2877
https://hg.mozilla.org/mozilla-central/rev/eeb83afc429e
https://hg.mozilla.org/mozilla-central/rev/61dd98c602d8
https://hg.mozilla.org/mozilla-central/rev/c820d2e6087e
https://hg.mozilla.org/mozilla-central/rev/5fbcd90b2e8f