1690634 - Remove well-known and static string atom instance

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P3)

Description

[Tooru Fujisawa [:arai]]

If all operations on well-known/static ParserAtomEntry pointer outside of ParserAtomsTable can be moved inside ParserAtomTable (bug 1689892), we can remove well-known/static ParserAtomEntry instances and instead use only TaggedParserAtomIndex as reference to them.

so that:

• reduce rodata size
• reduce startup cost for initializing the well-known table
• enable adding more well-known/static
  • bug 1689434 (length-3 or more static strings)
  • make all atoms in self-hosted.js well-known

Comment 1

[Tooru Fujisawa [:arai]]

for static strings, the content can be encoded directly in the TaggedParserAtomIndex, so we don't need extra space.
for well-known, we'll need string constant in addition to atom index, to retrieve the content,
but that can be shared between VM well-known atoms or other static string constant.

Comment 2

[Tooru Fujisawa [:arai]]

Thinking a bit more about the merit:

ParserAtomEntry consists of:

• HashNumber hash_
• uint32_t length_
• uint32_t flags_
• trailing chars

even if we remove the entry instance, we need hash and the equivalent of trailing chars.
So, 8 bytes (length and flags) can be definitely reduced for each atom.

then, for well-known atoms, we already have const char js_FOO_str[];, so that can be shared.
https://searchfox.org/mozilla-central/rev/927e525f481a93a8f63d27a78ae6201e42b1b1fb/js/src/vm/JSAtom.cpp#220-222

Comment 3

[Tooru Fujisawa [:arai]]

bug 1689434 experiment proved that we can have TaggedParserAtomIndex without ParserAtomEntry instance.
the problem there were:

• cannot statically calculate/allocate hash
• atomization in toExistingJSAtom call

for well-known and length-1/2 case, we already calculate/allocate hash, and toExistingJSatom doesn't touch ParserAtomEntry instance but just returns from VM side's table,
so this should work.

Comment 4

[Tooru Fujisawa [:arai]]

https://treeherder.mozilla.org/jobs?repo=try&revision=8934f21512c29261e32a08e0490a4049f89ad881

the patch doesn't yet dedup well-known atoms' string in frontend and js_FOO_str.
I hoped compiler can do that automatically, but maybe I should explicitly point the same string.

Comment 5

[Tooru Fujisawa [:arai]]

maybe we can move the WellKnownStringInfo in the patch out of frontend, and use that also from JSAtom.cpp
WellKnownStringInfo also contains hash, so JSRuntime::initializeAtoms can use that info to skip hash calculation on runtime

Comment 6

[Tooru Fujisawa [:arai]]

Here's the plan:

1. Rewrite length-1/2 cases not to retrieve ParserAtom, but construct necessary info (content, length) in place
2. Add WellKnownAtomInfo that holds (content, length, hash) for each well-known atoms
3. Use WellKnownAtomInfo array to populate WellKnownParserAtoms::wellKnownMap_
4. Rewrite well-known atom case not to retrieve ParserAtom, but use WellKnownAtomInfo
5. Remove WellKnownParserAtoms_ROM and WellKnownParserAtoms's pointer fields
6. Dedup js_*_str and WellKnownAtomInfo content
7. Rewrite JSAtom common names initialization to use WellKnownAtomInfo array, with pre-calculated hash

Comment 7

[Tooru Fujisawa [:arai]]

https://treeherder.mozilla.org/jobs?repo=try&revision=8b46c56823f425931d40b7b228960d11cdf5198d

Comment 8

[Tooru Fujisawa [:arai]]

Attachment: Bug 1690634 - Part 1: Use range-specific methods.

Comment 9

[Tooru Fujisawa [:arai]]

Attachment: Bug 1690634 - Part 2: Do not reteieve atom entry for length-1/2 in ParserAtomsTable::length.

Depends on D104593

Comment 10

[Tooru Fujisawa [:arai]]

Attachment: Bug 1690634 - Part 3: Add ParserAtomsTable::{getLength1Content,getLength2Content} methods and use in ParserAtomsTable::{dump,dumpCharsNoQuote}.

Depends on D104594

Comment 11

[Tooru Fujisawa [:arai]]

Attachment: Bug 1690634 - Part 4: Add length-1/2-specific path for ParserAtomsTable::isIdentifier.

Depends on D104595

Comment 12

[Tooru Fujisawa [:arai]]

Attachment: Bug 1690634 - Part 5: Add length-1/2-specific path for ParserAtomsTable::isExtendedUnclonedSelfHostedFunctionName.

Depends on D104596

Comment 13

[Tooru Fujisawa [:arai]]

Attachment: Bug 1690634 - Part 6: Add length-1/2-specific path for ParserAtomsTable::isIndex. r=nbp!

Depends on D104597

Comment 14

[Tooru Fujisawa [:arai]]

Attachment: Bug 1690634 - Part 7: Add length-1/2-specific path for ParserAtomsTable::toNumber.

Depends on D104598

Comment 15

[Tooru Fujisawa [:arai]]

Attachment: Bug 1690634 - Part 8: Add length-1/2-specific path for ParserAtomsTable::toNewUTF8CharsZ.

Depends on D104599

Comment 16

[Tooru Fujisawa [:arai]]

Attachment: Bug 1690634 - Part 9: Add length-1/2-specific path for ParserAtomsTable::toPrintableString.

Depends on D104600

Comment 17

[Tooru Fujisawa [:arai]]

Attachment: Bug 1690634 - Part 10: Add length-1/2-specific path for ParserAtomsTable::toQuotedString.

Depends on D104601

Comment 18

[Tooru Fujisawa [:arai]]

Attachment: Bug 1690634 - Part 11: Add length-1/2-specific path for ParserAtomsTable::appendTo.

Depends on D104602

Comment 19

[Tooru Fujisawa [:arai]]

Attachment: Bug 1690634 - Part 12: Add length-1/2-specific path for ParserAtomsTable::isModuleExportName.

Depends on D104603

Comment 20

[Tooru Fujisawa [:arai]]

Attachment: Bug 1690634 - Part 13: Remove TaggedParserAtomIndex variant of getParserAtom.

Depends on D104604

Comment 21

[Tooru Fujisawa [:arai]]

Attachment: Bug 1690634 - Part 14: Move dumpCharsNoQuote for length-1/2 into separate static function, to use from stencil dump.

Depends on D104605

Comment 22

[Tooru Fujisawa [:arai]]

Attachment: Bug 1690634 - Part 15: Remove ParserAtomsTable::{getLength1Static,getLength2Static}.

Depends on D104606

Comment 23

[Tooru Fujisawa [:arai]]

Attachment: Bug 1690634 - Part 16: Add WellKnownAtomInfo and use it in WellKnownParserAtoms::wellKnownMap_ and ParserAtomsTable::{dump,dumpCharsNoQuote}. r=nbp!

WellKnownAtomInfo can be used also from JSAtom initialization later, so the
definition is placed in vm/WellKnownAtom.h, and also WellKnownAtomId is moved
there.

Depends on D104607

Comment 24

[Tooru Fujisawa [:arai]]

Attachment: Bug 1690634 - Part 17: Use GetWellKnownAtomInfo in ParserAtomsTable methods. r=nbp!

Depends on D104608

Comment 25

[Tooru Fujisawa [:arai]]

Attachment: Bug 1690634 - Part 18: Remove WellKnownParserAtoms_ROM. r=nbp!

Depends on D104609

Comment 26

[Tooru Fujisawa [:arai]]

Attachment: Bug 1690634 - Part 19: Move js_*_str to WellKnownAtom.h. r=nbp!

Depends on D104610

Comment 27

[Tooru Fujisawa [:arai]]

Attachment: Bug 1690634 - Part 20: Use pre-calculated hash in JSAtom common name initialization. r=nbp!

Depends on D104611

Comment 28

[Tooru Fujisawa [:arai]]

Attachment: Bug 1690634 - Part 21: Pre-calculate hash for symbol-related common names. r=nbp!

Depends on D104612

Comment 29

[Tooru Fujisawa [:arai]]

The result of size reduction

file	before	after	diff
js shell	40,429,520	40,365,856	-63,664
x86_64 dmg	77,814,414	77,743,517	-70,897
XUL	161,242,216	161,193,856	-48,360

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/218bb88bba9d
Part 1: Use range-specific methods. r=nbp
https://hg.mozilla.org/integration/autoland/rev/1c3cdfd28aff
Part 2: Do not reteieve atom entry for length-1/2 in ParserAtomsTable::length. r=nbp
https://hg.mozilla.org/integration/autoland/rev/0449ae9ec529
Part 3: Add ParserAtomsTable::{getLength1Content,getLength2Content} methods and use in ParserAtomsTable::{dump,dumpCharsNoQuote}. r=nbp
https://hg.mozilla.org/integration/autoland/rev/a8abd8e27509
Part 4: Add length-1/2-specific path for ParserAtomsTable::isIdentifier. r=nbp
https://hg.mozilla.org/integration/autoland/rev/8118bd961f9c
Part 5: Add length-1/2-specific path for ParserAtomsTable::isExtendedUnclonedSelfHostedFunctionName. r=nbp
https://hg.mozilla.org/integration/autoland/rev/6e986649b078
Part 6: Add length-1/2-specific path for ParserAtomsTable::isIndex. r=nbp
https://hg.mozilla.org/integration/autoland/rev/8bf00cffdeba
Part 7: Add length-1/2-specific path for ParserAtomsTable::toNumber. r=nbp
https://hg.mozilla.org/integration/autoland/rev/218133df75d4
Part 8: Add length-1/2-specific path for ParserAtomsTable::toNewUTF8CharsZ. r=nbp
https://hg.mozilla.org/integration/autoland/rev/e3c635019f2a
Part 9: Add length-1/2-specific path for ParserAtomsTable::toPrintableString. r=nbp
https://hg.mozilla.org/integration/autoland/rev/d85cd571f708
Part 10: Add length-1/2-specific path for ParserAtomsTable::toQuotedString. r=nbp
https://hg.mozilla.org/integration/autoland/rev/c73eb9133e56
Part 11: Add length-1/2-specific path for ParserAtomsTable::appendTo. r=nbp
https://hg.mozilla.org/integration/autoland/rev/833782d13263
Part 12: Add length-1/2-specific path for ParserAtomsTable::isModuleExportName. r=nbp
https://hg.mozilla.org/integration/autoland/rev/74cbe3f3c730
Part 13: Remove TaggedParserAtomIndex variant of getParserAtom. r=nbp
https://hg.mozilla.org/integration/autoland/rev/b2ba7e3b9245
Part 14: Move dumpCharsNoQuote for length-1/2 into separate static function, to use from stencil dump. r=nbp
https://hg.mozilla.org/integration/autoland/rev/52dd05ae7c18
Part 15: Remove ParserAtomsTable::{getLength1Static,getLength2Static}. r=nbp
https://hg.mozilla.org/integration/autoland/rev/d80f85078a7f
Part 16: Add WellKnownAtomInfo and use it in WellKnownParserAtoms::wellKnownMap_ and ParserAtomsTable::{dump,dumpCharsNoQuote}. r=nbp
https://hg.mozilla.org/integration/autoland/rev/a97f35aa5813
Part 17: Use GetWellKnownAtomInfo in ParserAtomsTable methods. r=nbp
https://hg.mozilla.org/integration/autoland/rev/6550f2638e10
Part 18: Remove WellKnownParserAtoms_ROM. r=nbp
https://hg.mozilla.org/integration/autoland/rev/11be0b92d7fa
Part 19: Move js_*_str to WellKnownAtom.h. r=nbp
https://hg.mozilla.org/integration/autoland/rev/8b5f8b69af1f
Part 20: Use pre-calculated hash in JSAtom common name initialization. r=nbp
https://hg.mozilla.org/integration/autoland/rev/b23da4ca0327
Part 21: Pre-calculate hash for symbol-related common names. r=nbp

Comment 31

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/218bb88bba9d
https://hg.mozilla.org/mozilla-central/rev/1c3cdfd28aff
https://hg.mozilla.org/mozilla-central/rev/0449ae9ec529
https://hg.mozilla.org/mozilla-central/rev/a8abd8e27509
https://hg.mozilla.org/mozilla-central/rev/8118bd961f9c
https://hg.mozilla.org/mozilla-central/rev/6e986649b078
https://hg.mozilla.org/mozilla-central/rev/8bf00cffdeba
https://hg.mozilla.org/mozilla-central/rev/218133df75d4
https://hg.mozilla.org/mozilla-central/rev/e3c635019f2a
https://hg.mozilla.org/mozilla-central/rev/d85cd571f708
https://hg.mozilla.org/mozilla-central/rev/c73eb9133e56
https://hg.mozilla.org/mozilla-central/rev/833782d13263
https://hg.mozilla.org/mozilla-central/rev/74cbe3f3c730
https://hg.mozilla.org/mozilla-central/rev/b2ba7e3b9245
https://hg.mozilla.org/mozilla-central/rev/52dd05ae7c18
https://hg.mozilla.org/mozilla-central/rev/d80f85078a7f
https://hg.mozilla.org/mozilla-central/rev/a97f35aa5813
https://hg.mozilla.org/mozilla-central/rev/6550f2638e10
https://hg.mozilla.org/mozilla-central/rev/11be0b92d7fa
https://hg.mozilla.org/mozilla-central/rev/8b5f8b69af1f
https://hg.mozilla.org/mozilla-central/rev/b23da4ca0327