1847677 - Add well-known-ids variant to JSPropertySpec

Status: ASSIGNED

(Core :: JavaScript Engine, task, P3)

Description

[Tooru Fujisawa [:arai]]

If a JSPropertySpec contains a name that's well-known-id, putting WellKnownAtomId into JSPropertySpec allows the atomization be done with GetWellKnownAtom call.

Comment 1

[Tooru Fujisawa [:arai]]

Attachment: Bug 1847677 - Part 1: Move CommonPropertyNames.h into js/public/friend. r?jandem!

In order to expose js::WellKnownAtomId as part of JSPropertySpec::Name,
the underlying data source in CommonPropertyNames.h also needs to be exposed.

Depends on D185730

Comment 2

[Tooru Fujisawa [:arai]]

Attachment: Bug 1847677 - Part 2: Move WellKnownAtomId into js/public/friend/WellKnownAtomId.h. r?jandem!

Depends on D185731

Comment 3

[Tooru Fujisawa [:arai]]

Attachment: Bug 1847677 - Part 3: Add js::WellKnownAtomId variant to JSPropertySpec::Name. r?jandem!

Depends on D185732

Comment 4

[Tooru Fujisawa [:arai]]

Attachment: Bug 1847677 - Part 4: Use js::WellKnownAtomId variant of JSPropertySpec where possible. r?jandem!

Depends on D185733

Comment 5

[Tooru Fujisawa [:arai]]

Attachment: Bug 1847677 - Part 5: Use js::WellKnownAtomId variant of JSPropertySpec in debugger. r?jandem!

Depends on D185734

Comment 6

[Jan de Mooij [:jandem]]

What percentage of JSPropertySpec names that we atomize at runtime can now use the well known atom id?

I'm wondering if this is worth doing with the current set, or if we should think about implementing global static atoms first so we can include ~all names in that?

Comment 7

[Tooru Fujisawa [:arai]]

it's applied to 473 specs which is 34% of SpiderMonkey internal, but only 2% in the entire browser, given most specs are auto-generated and outside of SpiderMonkey.

(In reply to Tooru Fujisawa [:arai] from bug 1847672 comment #10)

there are 3,632 static JSPropertySpec or JSFunctionSpec arrays (206 in SpiderMonkey, and remaining in bindings),
and 20,288 static JSPropertySpec or JSFunctionSpec instances there (1,394 in SpiderMonkey, and remaining in bindings).

so, yeah, having the equivalent of well-known atom outside of SpiderMonkey will make this more effectful.

Comment 8

[Markus Stange [:mstange]]

(In reply to Tooru Fujisawa [:arai] from comment #7)

so, yeah, having the equivalent of well-known atom outside of SpiderMonkey will make this more effectful.

Would this help bug 1846110? https://share.firefox.dev/3seR1lp spends a fair amount of time in PropertySpecNameToId.

Comment 9

[Tooru Fujisawa [:arai]]

(In reply to Markus Stange [:mstange] from comment #8)

(In reply to Tooru Fujisawa [:arai] from comment #7)

so, yeah, having the equivalent of well-known atom outside of SpiderMonkey will make this more effectful.

Would this help bug 1846110? https://share.firefox.dev/3seR1lp spends a fair amount of time in PropertySpecNameToId.

Yes, bug 1848278 is going to make the well-known atom extensible, and that will move the atomization from each JS_DefineProperties to initialization step in JS::InitSelfHostedCode, with statically calculating the length and hash.

Comment 10

[Tooru Fujisawa [:arai]]

Attachment: Bug 1847677 - Part 0: Simplify JSFunctionSpec macro. r?jandem!

Comment 11

[Tooru Fujisawa [:arai]]

Attachment: Bug 1847677 - Part 1: Add JS::GetWellKnownAtom and JS::GetWellKnownAtomKey. r?jandem!

Depends on D186305

Comment 12

[Tooru Fujisawa [:arai]]

Attachment: Bug 1847677 - Part 2: Add JS::WellKnownAtomId variant to JSPropertySpec::Name. r?jandem!

Depends on D186306

Comment 13

[Tooru Fujisawa [:arai]]

Attachment: Bug 1847677 - Part 3: Sort CommonPropertyNames. r?jandem!

Alphabetical order, ignore case, with an exception for *8, *16 *32 suffixes

Depends on D186307

Comment 14

[Tooru Fujisawa [:arai]]

Attachment: Bug 1847677 - Part 4: Use JS::WellKnownAtomId variant of JSPropertySpec/JSFunctionSpec in Array, BigInt, Boolean, Number, Object, RegExp, String, and Symbol. r?jandem!

Depends on D186308

Comment 15

[Tooru Fujisawa [:arai]]

Attachment: Bug 1847677 - Part 5: Use JS::WellKnownAtomId variant of JSPropertySpec/JSFunctionSpec in Map, Set, WeakMap, WeakSet. r?jandem!

Depends on D186309

Comment 16

[Tooru Fujisawa [:arai]]

Attachment: Bug 1847677 - Part 6: Use JS::WellKnownAtomId variant of JSPropertySpec/JSFunctionSpec in Date, Math, JSON, and Errors. r?jandem!

Depends on D186310

Comment 17

[Tooru Fujisawa [:arai]]

Attachment: Bug 1847677 - Part 7: Use JS::WellKnownAtomId variant of JSPropertySpec/JSFunctionSpec in functions and Promise. r?jandem!

Depends on D186311

Comment 18

[Tooru Fujisawa [:arai]]

Attachment: Bug 1847677 - Part 8: Use JS::WellKnownAtomId variant of JSPropertySpec/JSFunctionSpec in TypedArrays. r?jandem!

Depends on D186312

Comment 19

[Tooru Fujisawa [:arai]]

Attachment: Bug 1847677 - Part 9: Use JS::WellKnownAtomId variant of JSPropertySpec/JSFunctionSpec in Reflect, Proxy, and ShadowRealm. r?jandem!

Depends on D186313

Comment 20

[Tooru Fujisawa [:arai]]

Attachment: Bug 1847677 - Part 10: Use JS::WellKnownAtomId variant of JSPropertySpec/JSFunctionSpec in WeakRef. r?jandem!

Depends on D186314

Comment 21

[Tooru Fujisawa [:arai]]

Attachment: Bug 1847677 - Part 11: Use JS::WellKnownAtomId variant of JSPropertySpec/JSFunctionSpec in Intl. r?jandem!

Depends on D186315

Comment 22

[Tooru Fujisawa [:arai]]

Attachment: Bug 1847677 - Part 12: Use JS::WellKnownAtomId variant of JSPropertySpec/JSFunctionSpec in WebAssembly. r?jandem!

Depends on D186316

Comment 23

[Tooru Fujisawa [:arai]]

Attachment: Bug 1847677 - Part 13: Use JS::WellKnownAtomId variant of JSPropertySpec/JSFunctionSpec in Temporal. r?jandem!

Depends on D186317

Comment 24

[Tooru Fujisawa [:arai]]

Attachment: Bug 1847677 - Part 14: Use JS::WellKnownAtomId variant of JSPropertySpec/JSFunctionSpec in Tuple. r?jandem!

Depends on D186318

Comment 25

[Tooru Fujisawa [:arai]]

Attachment: Bug 1847677 - Part 15: Use JS::WellKnownAtomId variant of JSPropertySpec/JSFunctionSpec in Debugger where possible. r?jandem!

Depends on D186319

Comment 26

[Tooru Fujisawa [:arai]]

self-hosting intrinsics needs some more work to use the JS::WellKnownAtomId, because it currently uses the string content for search.

https://searchfox.org/mozilla-central/rev/f1f50693655c093d974f026bd37860d939cd5529/js/src/vm/SelfHosting.cpp#1893

static const JSFunctionSpec intrinsic_functions[] = {

https://searchfox.org/mozilla-central/rev/f1f50693655c093d974f026bd37860d939cd5529/js/src/vm/SelfHosting.cpp#2297-2299

static void CheckSelfHostedIntrinsics() {
  // The `intrinsic_functions` list must be sorted so that we can use
  // mozilla::BinarySearch to do lookups on demand.

https://searchfox.org/mozilla-central/rev/f1f50693655c093d974f026bd37860d939cd5529/js/src/vm/SelfHosting.cpp#2343

const JSFunctionSpec* js::FindIntrinsicSpec(js::PropertyName* name) {

Comment 27

[Tooru Fujisawa [:arai]]

Attachment: Bug 1847677 - Part 16: Use JS::WellKnownAtomId variant of JSPropertySpec/JSFunctionSpec in self-hosting intrinsics. r?jandem!

Also add JSOp::GetIntrinsicFunc to use the index into intrinsic_functions:

• JSOp::GetIntrinsicFunc is 2 bytes shorter than JSOp::GetIntrinsic
• JSOp::GetIntrinsicFunc doesn't perform binary search during runtime

Comment 28

[Tooru Fujisawa [:arai]]

Attachment: Bug 1847677 - Part 17: Use JS::WellKnownAtomId variant of JSPropertySpec/JSFunctionSpec in js-ctypes where possible. r?jandem!

Depends on D186441