1687428 - Use TaggedParserAtomIndex everywhere and remove ParserAtomEntry::index_ field

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P1)

Description

[Tooru Fujisawa [:arai]]

ParserAtomEntry contains index_ field, to get TaggedParserAtomIndex from ParserAtom pointer.
this is used while converting frontend-internal data to stencil, and also used while stencil instantiation/XDR.

If we can replace all ParserAtom* fields/variables with TaggedParserAtomIndex, we get:

• on-memory size reduction, especially in ParseNode
• single ParserAtomEntry instance can be shared between multiple tables
  • helps bug 1687094, for XDR data size reduction

Comment 1

[Tooru Fujisawa [:arai]]

The plan is the following:

1. rewrite each ParserAtom consumer class to receive/return TaggedParserAtomIndex instead of ParserAtom/ParserName
   • this happens gradually and the intermediate state may be somewhat unoptimal
   • at the end of this stage, ParserAtomEntry::toIndex should be called only after invoking ParserAtomsTable methods
2. make ParserAtomsTable return TaggedParserAtomIndex instead of ParserAtom/ParserName
   • after this stage, there's only "TaggedParserAtomIndex -> ParserAtom" conversion, and no "ParserAtom -> TaggedParserAtomIndex" conversion
3. remove ParserAtomEntry::index_ field and ParserAtomEntry::toIndex

Comment 2

[Tooru Fujisawa [:arai]]

This seems to work. I'll look into bug 1687094 after this.

Comment 3

[Tooru Fujisawa [:arai]]

With the raptor yahoo-mail, mail_app_es6_09* JS file's XDR,
my local patch stack reduces the size of atom table part from 1,977,500 bytes to 1,776,884 bytes,
so 200,616 bytes (10%) reduction (~50000 atoms * 4 bytes per each index)

Also this enables us to dedup ParserAtomEntry inside XDR, so we can expect further reduction :)

Comment 4

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687428 - Part 1: Add TaggedParserAtomIndex-specific accessor to ParserNode and FunctionBox. r?nbp!

To gradually replace all consumers, provide both ParserAtom/ParserName accessor
and TaggedParserAtomIndex accessor.

ParserAtom/ParserName accessor will be removed later, and TaggedParserAtomIndex
accessor's Index suffix will be removed later.

Comment 5

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687428 - Part 2: Split tagged index raw data accessor into mutable and immutable. r?nbp!

Depends on D102532

Comment 6

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687428 - Part 3: Use TaggedParserAtomIndex in frontend::GCThingList. r?nbp!

Also add ParserAtomsTable::markUsedByStencil method to hide ParserAtom pointer.

Depends on D102533

Comment 7

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687428 - Part 4: Move well-known TaggedParserAtomIndex methods into dedicate class. r?nbp!

Given TaggedParserAtomIndex::null() isn't well-known "null",
we should split the namespace.

Also, now that there's no risk of name collision, populate all well-known
indices.

Depends on D102534

Comment 8

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687428 - Part 5: Use TaggedParserAtomIndex in RecyclableNameMap. r?nbp!

Added TrivialTaggedParserAtomIndex to use TaggedParserAtomIndex in collection
that requires trivial type.

Also move TaggedParserAtomIndexHasher into its own header,
with TrivialTaggedParserAtomIndexHasher, to share between multiple consumers.

Depends on D102535

Comment 9

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687428 - Part 6: Use TaggedParserAtomIndex in AtomVector. r?nbp!

Because of VectorPool requirement, the element needs to be pointer size,
so add TrivialPtrSizeTaggedParserAtomIndex, that uses uintptr_t instead of
uint32_t.

Depends on D102536

Comment 10

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687428 - Part 7: Use TaggedParserAtomIndex in EmitterScope. r?nbp!

Depends on D102537

Comment 11

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687428 - Part 8: Use TaggedParserAtomIndex in LabelControl. r?nbp!

Depends on D102538

Comment 12

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687428 - Part 9: Use TaggedParserAtomIndex in BytecodeEmitter. r?nbp!

Depends on D102539

Comment 13

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687428 - Part 10: Use TaggedParserAtomIndex in emitter helpers. r?nbp!

Depends on D102540

Comment 14

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687428 - Part 11: Use TaggedParserAtomIndex in ModuleBuilder. r?nbp!

Depends on D102541

Comment 15

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687428 - Part 12: Use TaggedParserAtomIndex in AsmJS. r?nbp!

Depends on D102542

Comment 16

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687428 - Part 13: Remove ParserAtom/ParserName accessor from ParseNode. r?nbp!

IsEscapeFreeStringLiteral is split to each useStrict/useAsm case, in order to
avoid accessing ParserAtom content.

Depends on D102543

Comment 17

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687428 - Part 14: Remove ParserAtom/ParserName from ParseNode and ParseHandler. r?nbp!

Now NameNode and LoopControlStatement uses TaggedParserAtomIndex in its field.

ParseNode::dump is rewritten to receive ParserBase, to get ParserAtomsTable
access, in order to dump not-well-known atom content.
It also works without ParserAtomsTable (in case ParseNode::dump is directly
called from debugger), but in that case not-well-known is dumped as index.

Depends on D102544

Comment 18

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687428 - Part 15: Rename TaggedParserAtomIndex-specific accessor in ParseNode. r?nbp!

Depends on D102545

Comment 19

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687428 - Part 16: Remove ParserAtom/ParserName from SharedContext/FunctionBox. r?nbp!

Depends on D102546

Comment 20

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687428 - Part 17: Rename TaggedParserAtomIndex-specific accessor in FunctionBox. r?nbp!

Depends on D102547

Comment 21

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687428 - Part 18: Remove ParserAtom/ParserName from Token. r?nbp!

Also renames TaggedParserAtomIndex-specific accessor to remove Index suffix.

TokenStreamAnyChars::currentName receives ParserAtomsTable to get
ParserAtom, but this change is temporary and will be reverted when removing
ParserAtom/ParserName from TokenStream.

Depends on D102548

Comment 22

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687428 - Part 19: Remove ParserAtom/ParserName from TokenStream methods. r?nbp!

Depends on D102549

Comment 23

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687428 - Part 20: Remove ParserAtom/ParserName from ParseContext. r?nbp!

Depends on D102550

Comment 24

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687428 - Part 21: Remove unused liftParserAtomToJSAtom method from EitherParser. r?nbp!

Depends on D102551

Comment 25

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687428 - Part 22: Remove ParserAtom/ParserName from Parser. r?nbp!

There's inconsistency around compilationState_.getParserAtomAt vs
compilationState_.parserAtoms.getParserAtom.
This will be solved later.

Also removed unused/not-existent functions liftParserAtomToJSAtom and
newFunction.

Depends on D102552

Comment 26

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687428 - Part 23: Remove ParserAtom/ParserName from RealmInstrumentation. r?nbp!

Depends on D102553

Comment 27

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687428 - Part 24: Remove ParserAtom/ParserName return type from StringBuffer. r?nbp!

Depends on D102554

Comment 28

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687428 - Part 25: Remove ParserAtom/ParserName from jsnum. r?nbp!

Depends on D102555

Comment 29

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687428 - Part 26: Remove ParserAtom/ParserName from Frontend2. r?nbp!

Depends on D102556

Comment 30

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687428 - Part 27: Remove remaining unnecessary ParserAtomEntry::toIndex callsite. r?nbp!

Depends on D102557

Comment 31

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687428 - Part 28: Store TaggedParserAtomIndex in ParserAtomsTable. r?nbp!

As a preparation to remove index_ field from ParserAtomEntry,
ParserAtomsTable should maintain a map from ParserAtomEntry pointer to
TaggedParserAtomIndex.

Depends on D102558

Comment 32

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687428 - Part 29: Return TaggedParserAtomIndex from ParserAtomsTable::concatAtoms. r?nbp!

ParserAtomsTable::concatAtoms still receives ParserAtom* range, because we need
character access, and InflatedChar16Sequence needs ParserAtom* range.

Depends on D102559

Comment 33

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687428 - Part 30: Return TaggedParserAtomIndex from ParserAtomsTable::intern*. r?nbp!

Depends on D102560

Comment 34

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687428 - Part 31: Return TaggedParserAtomIndex from WellKnownParserAtoms::lookup*. r?nbp!

Depends on D102561

Comment 35

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687428 - Part 32: Hide ParserAtomEntry index accessors. r?nbp!

As a preparation to remove index_ field, rewrite its consumer to use different
way to get index.

Also, ParserAtomEntry::{toJSAtom, instantiate} receive TaggedParserAtomIndex,
to avoid using its field, but with an assertion for now.
The assertion will be removed later.

In InstantiateMarkedAtoms and XDRParserAtomTable, the atom's index matches the
item's index inside the span, and isUsedByStencil is set only for
not-well-known atoms, so we can just check isUsedByStencil and then use
the index inside the span.

Depends on D102562

Comment 36

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687428 - Part 33: Remove most of ParserAtomEntry index accessors. r?nbp!

Depends on D102563

Comment 37

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687428 - Part 34: Add WellKnownOrStaticFlag to ParserAtomEntry. r?nbp!

ParserAtomEntry::index_ field is used also to distinguish well-known/static.
We need separate flag, in order to remove the field.

Depends on D102564

Comment 38

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687428 - Part 35: Remove ParserAtomEntry::index_ field. r?nbp!

Depends on D102565

Comment 39

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687428 - Part 36: Remove CompilationState::getParserAtomAt. r?nbp!

Because CompilationState.parserAtoms.getParserAtom provides the same thing,
removed CompilationState::getParserAtomAt.

Depends on D102566

Comment 40

[Tooru Fujisawa [:arai]]

Attachment: Bug 1687428 - Part 37: Move getParserAtom into ParserAtomToPrintableString. r?nbp!

Depends on D102567

Comment 41

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a295d374f63c
Part 1: Add TaggedParserAtomIndex-specific accessor to ParserNode and FunctionBox. r=nbp
https://hg.mozilla.org/integration/autoland/rev/cf923036a361
Part 2: Split tagged index raw data accessor into mutable and immutable. r=nbp
https://hg.mozilla.org/integration/autoland/rev/21dae3d1ea01
Part 3: Use TaggedParserAtomIndex in frontend::GCThingList. r=nbp
https://hg.mozilla.org/integration/autoland/rev/f11873f0827c
Part 4: Move well-known TaggedParserAtomIndex methods into dedicate class. r=nbp
https://hg.mozilla.org/integration/autoland/rev/721a764c88ea
Part 5: Use TaggedParserAtomIndex in RecyclableNameMap. r=nbp
https://hg.mozilla.org/integration/autoland/rev/48bc889e7f9b
Part 6: Use TaggedParserAtomIndex in AtomVector. r=nbp
https://hg.mozilla.org/integration/autoland/rev/178975ab315b
Part 7: Use TaggedParserAtomIndex in EmitterScope. r=nbp
https://hg.mozilla.org/integration/autoland/rev/02c07b673b6a
Part 8: Use TaggedParserAtomIndex in LabelControl. r=nbp
https://hg.mozilla.org/integration/autoland/rev/2b773e770cf6
Part 9: Use TaggedParserAtomIndex in BytecodeEmitter. r=nbp
https://hg.mozilla.org/integration/autoland/rev/d581c6939b6e
Part 10: Use TaggedParserAtomIndex in emitter helpers. r=nbp
https://hg.mozilla.org/integration/autoland/rev/d53977dc05a6
Part 11: Use TaggedParserAtomIndex in ModuleBuilder. r=nbp
https://hg.mozilla.org/integration/autoland/rev/a3cbfc45580b
Part 12: Use TaggedParserAtomIndex in AsmJS. r=nbp
https://hg.mozilla.org/integration/autoland/rev/14fc717dea9a
Part 13: Remove ParserAtom/ParserName accessor from ParseNode. r=nbp
https://hg.mozilla.org/integration/autoland/rev/b55652713ba2
Part 14: Remove ParserAtom/ParserName from ParseNode and ParseHandler. r=nbp
https://hg.mozilla.org/integration/autoland/rev/08e996759dd9
Part 15: Rename TaggedParserAtomIndex-specific accessor in ParseNode. r=nbp
https://hg.mozilla.org/integration/autoland/rev/e53c556bba8c
Part 16: Remove ParserAtom/ParserName from SharedContext/FunctionBox. r=nbp
https://hg.mozilla.org/integration/autoland/rev/c2f48e868180
Part 17: Rename TaggedParserAtomIndex-specific accessor in FunctionBox. r=nbp
https://hg.mozilla.org/integration/autoland/rev/e3b13445f55a
Part 18: Remove ParserAtom/ParserName from Token. r=nbp
https://hg.mozilla.org/integration/autoland/rev/b722d3036840
Part 19: Remove ParserAtom/ParserName from TokenStream methods. r=nbp
https://hg.mozilla.org/integration/autoland/rev/0810505263e6
Part 20: Remove ParserAtom/ParserName from ParseContext. r=nbp
https://hg.mozilla.org/integration/autoland/rev/48bf6019c703
Part 21: Remove unused liftParserAtomToJSAtom method from EitherParser. r=nbp
https://hg.mozilla.org/integration/autoland/rev/73ea6560f99d
Part 22: Remove ParserAtom/ParserName from Parser. r=nbp
https://hg.mozilla.org/integration/autoland/rev/b8318cf13b27
Part 23: Remove ParserAtom/ParserName from RealmInstrumentation. r=nbp
https://hg.mozilla.org/integration/autoland/rev/1b8f396bdcb5
Part 24: Remove ParserAtom/ParserName return type from StringBuffer. r=nbp
https://hg.mozilla.org/integration/autoland/rev/08b75ff379d9
Part 25: Remove ParserAtom/ParserName from jsnum. r=nbp
https://hg.mozilla.org/integration/autoland/rev/fc49babba848
Part 26: Remove ParserAtom/ParserName from Frontend2. r=nbp
https://hg.mozilla.org/integration/autoland/rev/b89b5f731712
Part 27: Remove remaining unnecessary ParserAtomEntry::toIndex callsite. r=nbp
https://hg.mozilla.org/integration/autoland/rev/2fa38f16f81a
Part 28: Store TaggedParserAtomIndex in ParserAtomsTable. r=nbp
https://hg.mozilla.org/integration/autoland/rev/21b2349db370
Part 29: Return TaggedParserAtomIndex from ParserAtomsTable::concatAtoms. r=nbp
https://hg.mozilla.org/integration/autoland/rev/5d9fb3328608
Part 30: Return TaggedParserAtomIndex from ParserAtomsTable::intern*. r=nbp
https://hg.mozilla.org/integration/autoland/rev/cbfc7ec206b4
Part 31: Return TaggedParserAtomIndex from WellKnownParserAtoms::lookup*. r=nbp
https://hg.mozilla.org/integration/autoland/rev/7e28576fa2f3
Part 32: Hide ParserAtomEntry index accessors. r=nbp
https://hg.mozilla.org/integration/autoland/rev/bb45134f6099
Part 33: Remove most of ParserAtomEntry index accessors. r=nbp
https://hg.mozilla.org/integration/autoland/rev/ff9cadc682f7
Part 34: Add WellKnownOrStaticFlag to ParserAtomEntry. r=nbp
https://hg.mozilla.org/integration/autoland/rev/c1189f833fbc
Part 35: Remove ParserAtomEntry::index_ field. r=nbp
https://hg.mozilla.org/integration/autoland/rev/ba478cc96ac8
Part 36: Remove CompilationState::getParserAtomAt. r=nbp
https://hg.mozilla.org/integration/autoland/rev/44155b6558c5
Part 37: Move getParserAtom into ParserAtomToPrintableString. r=nbp

Comment 42

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/a295d374f63c
https://hg.mozilla.org/mozilla-central/rev/cf923036a361
https://hg.mozilla.org/mozilla-central/rev/21dae3d1ea01
https://hg.mozilla.org/mozilla-central/rev/f11873f0827c
https://hg.mozilla.org/mozilla-central/rev/721a764c88ea
https://hg.mozilla.org/mozilla-central/rev/48bc889e7f9b
https://hg.mozilla.org/mozilla-central/rev/178975ab315b
https://hg.mozilla.org/mozilla-central/rev/02c07b673b6a
https://hg.mozilla.org/mozilla-central/rev/2b773e770cf6
https://hg.mozilla.org/mozilla-central/rev/d581c6939b6e
https://hg.mozilla.org/mozilla-central/rev/d53977dc05a6
https://hg.mozilla.org/mozilla-central/rev/a3cbfc45580b
https://hg.mozilla.org/mozilla-central/rev/14fc717dea9a
https://hg.mozilla.org/mozilla-central/rev/b55652713ba2
https://hg.mozilla.org/mozilla-central/rev/08e996759dd9
https://hg.mozilla.org/mozilla-central/rev/e53c556bba8c
https://hg.mozilla.org/mozilla-central/rev/c2f48e868180
https://hg.mozilla.org/mozilla-central/rev/e3b13445f55a
https://hg.mozilla.org/mozilla-central/rev/b722d3036840
https://hg.mozilla.org/mozilla-central/rev/0810505263e6
https://hg.mozilla.org/mozilla-central/rev/48bf6019c703
https://hg.mozilla.org/mozilla-central/rev/73ea6560f99d
https://hg.mozilla.org/mozilla-central/rev/b8318cf13b27
https://hg.mozilla.org/mozilla-central/rev/1b8f396bdcb5
https://hg.mozilla.org/mozilla-central/rev/08b75ff379d9
https://hg.mozilla.org/mozilla-central/rev/fc49babba848
https://hg.mozilla.org/mozilla-central/rev/b89b5f731712
https://hg.mozilla.org/mozilla-central/rev/2fa38f16f81a
https://hg.mozilla.org/mozilla-central/rev/21b2349db370
https://hg.mozilla.org/mozilla-central/rev/5d9fb3328608
https://hg.mozilla.org/mozilla-central/rev/cbfc7ec206b4
https://hg.mozilla.org/mozilla-central/rev/7e28576fa2f3
https://hg.mozilla.org/mozilla-central/rev/bb45134f6099
https://hg.mozilla.org/mozilla-central/rev/ff9cadc682f7
https://hg.mozilla.org/mozilla-central/rev/c1189f833fbc
https://hg.mozilla.org/mozilla-central/rev/ba478cc96ac8
https://hg.mozilla.org/mozilla-central/rev/44155b6558c5