1850344 - Use permanent atom for startup-related or frequently-used atoms, or all atoms used by bindings

Status: ASSIGNED

(Core :: DOM: Bindings (WebIDL), task, P3)

Description

[Tooru Fujisawa [:arai]]

bug 1848278 is going to make the permanent atom extensible from embedder, and which allows optimizing CreateInterfaceObject and some other atom-related operation.
with speedometer 3, the 30% of the time taken by CreateInterfaceObject is atomization, and that part can be moved to compile-time (hash and length calculation, getter/setter name calculation) + startup (remaining atomization operation into permanent atoms)

Comment 1

[Tooru Fujisawa [:arai]]

WIP patch stack: https://treeherder.mozilla.org/jobs?repo=try&revision=5d0a314f90f54b4197a10a6cf03df150c8b10be5

Comment 2

[Peter Van der Beken [:peterv]]

Before WebIDL bindings we used to selectively optimize a lot of the DOM bindings (cf. wrapper cache, slim wrappers, quickstubs, …). The problem is that that easily leads to performance cliffs, where once a new DOM API becomes popular the performance suddenly plummets because it wasn't optimized (because it used to be rarely used). These days we try to avoid that (even though we have some mechanisms like caching of attribute values that still are opt-in).

Did we evaluate not making this opt-in, but doing it for all interfaces/properties? It would avoid having another list to keep up-to-date when adding or removing interfaces, turning on prefs, … In my experience these kind of lists get quickly out-of-date.

Comment 3

[Tooru Fujisawa [:arai]]

the problem with the current implementation is that some part of atomization (allocation, putting into table) is done at startup instead of compile-time, while it had been lazily done when the interface is first used, so this can lead to slow startup if we have so much items.
I'll check how much it increases if we put all names into permanent atoms.
maybe we need to move more parts into compile-time and possibly optimize the JSAtom representation for this purpose.

Comment 4

[Tooru Fujisawa [:arai]]

Atomization at startup takes ~800us with 12409 entries (all WebIDL properties and interface names), that would be acceptable?
Also, the binary size increases by 360kB with the current representation.

Comment 5

[Andrew McCreight [:mccr8]]

360kB is a fair amount. Have you tried any kind of deduplication? That saved a lot of binary size for the XPIDL string constants (in xptcodegen.py).

Comment 6

[Tooru Fujisawa [:arai]]

deduplication isn't yet performed.

currently the getter name, setter name, and original property name are stored separately, but at least either getter or setter name and the original property name can be deduplicated:

get alignContent\0
^   ^
|   |
|   property name
getter name

or maybe the getter/setter name concatenation could be done at runtime, while still pre-calculating the hash at compile-time.
I'll look into deduplication, and also optimizing the storage

Comment 7

[Tooru Fujisawa [:arai]]

if we compress data and calculate some part at startup (accessor name etc), the essential increase from the current state would be length+hash+pointer or index, which would be ~100kB.
I'll look into the details of the trade off between the binary size and startup cost.

Comment 8

[Tooru Fujisawa [:arai]]

bug 1858040 solved the situation by making the accessor name calculation more lazy.
now the necessary static data is only for the property name, and the increased data is only its length and hash and pointer.
I'll update the statistics shortly.

Comment 9

[Tooru Fujisawa [:arai]]

With only property names (without the compression for length/index), the XUL file size increase is ~110kB.
There are 6782 items, and the increase matches to the length+hash+pointer which takes ~105kB.

Comment 10

[Tooru Fujisawa [:arai]]

Then, considering the compression, the method names and interface names are referred also from other places, for example from profiler marker code:

https://searchfox.org/mozilla-central/source/__GENERATED__/__macosx64__/dom/bindings/ChromeUtilsBinding.cpp#6493-6494

AUTO_PROFILER_LABEL_DYNAMIC_FAST(
  "ChromeUtils", "aliveUtilityProcesses", DOM, cx,

If we're to replace string pointer with "index into large string", we need to:

• make sure the compiler's/linker's de-duplication works even between small string vs large string
  • at least the large string should be have null-terminator for each item
• rewrite those other references to explicitly use the substring of the large string

otherwise we'll have duplicate text content for each method and interface name, and the binary size increases.

Comment 11

[Tooru Fujisawa [:arai]]

Attachment: WIP: Bug 1850344 - Part 1: Make well-known atoms used by embedding not-conditional.

In order to simplify the codegen, stop using IF_* macro for well-knonw atoms
which is used by WebIDL.

Depends on D186329

Comment 12

[Tooru Fujisawa [:arai]]

Attachment: WIP: Bug 1850344 - Part 2: Use well-known atoms and extended known atoms in WebIDL bindings methods and attributes.

Depends on D192417

Comment 13

[Tooru Fujisawa [:arai]]

Attachment: WIP: Bug 1850344 - Part 3: Use extended known atoms in WebIDL interface names.

Depends on D192418

Comment 14

[Tooru Fujisawa [:arai]]

Attachment: WIP: Bug 1850344 - Part 4: Use extended known atoms in WebIDL RegisterBindings.

Depends on D192419

Comment 15

[Tooru Fujisawa [:arai]]

Attachment: WIP: Bug 1850344 - Part 5: Use JS::WellKnownAtomId in XrayOwnNativePropertyKeys.

Depends on D192420

Comment 16

[Tooru Fujisawa [:arai]]

Attachment: WIP: Bug 1850344 - Part 6: Use JS::WellKnownAtomId in DefineStaticJSVals.

Depends on D192421

Comment 17

[Tooru Fujisawa [:arai]]

Attachment: WIP: Bug 1850344 - Part 7: Use JSPropertySpec::Name in ConstantSpec.

Depends on D192422

Comment 18

[Tooru Fujisawa [:arai]]

Attachment: WIP: Bug 1850344 - Part 8: Use JS::WellKnownAtomId in CreateInterfaceObjects.

Depends on D192423

Comment 19

[Tooru Fujisawa [:arai]]

Attachment: WIP: Bug 1850344 - Part 9: Use JS::WellKnownAtomId in GetDesiredProto.

Depends on D192424

Comment 20

[Tooru Fujisawa [:arai]]

Attachment: WIP: Bug 1850344 - Part 10: Add isInstance to nsExtendedKnownAtoms and use it in CreateInterfaceObject.

Depends on D192425

Comment 21

[Tooru Fujisawa [:arai]]

Attachment: WIP: Bug 1850344 - Part 11: Use JSPropertySpec::Name for unscopableNames.

Depends on D192426

Comment 22

[Tooru Fujisawa [:arai]]

Attachment: WIP: Bug 1850344 - Part 12: Use JSPropertySpec::Name for legacyWindowAliases.

Depends on D192427

Comment 23

[Tooru Fujisawa [:arai]]

Attachment: WIP: Bug 1850344 - Part 13: Use JSPropertySpec::Name in LegacyFactoryFunction.

Depends on D192428