1898153 - Refactor Code/CodeTier/etc to be ready for partial tiering

Status: RESOLVED FIXED

(Core :: JavaScript: WebAssembly, enhancement, P3)

Description

[Julian Seward [:jseward]]

Refactor Code/CodeTier/etc to be ready for partial tiering. This is a
follow-on from bug 1891182.

Comment 1

[Ryan Hunt [:rhunt]]

Attachment: Bug 1898153 - wasm: Have only one LazyStubTier and put it on Code. r?jseward

LazyStubTier contains the JIT code for lazy stubs. It is dynamically
grow-able as new stubs are generated. Right now each LST contains
only stubs for a single tier of code, hence it lives on CodeTier.

This commit changes this so that LST contains code from either
tier, and so can live on Code instead of CodeTier. This removes
the need to have complex locking logic when committing a new
tier, as it needed to try to acquire the per-tier lock in a
careful order.

There's no issue with the different tiers intermingling in the
same JIT pages, as we never serialize them or free them.

The class is not renamed from LazyStubTier to LazyStubs (as you
might expect) as the class wil be removed in a later patch.

Comment 2

[Ryan Hunt [:rhunt]]

Attachment: Bug 1898153 - wasm: Merge MetadataTier into CodeTier. r?jseward

MetadataTier and CodeTier are 1:1, and so there's no point
in having separate classes for them. This patch merges them
to reduce the number of concepts in our code hierarchy to
worry about. Some references to metadataTier still remain,
but will be cleaned up in future patches.

Depends on D213137

Comment 3

[Ryan Hunt [:rhunt]]

Attachment: Bug 1898153 - wasm: Rename CodeTier to CodeBlock. r?jseward

CodeTier contains JIT code for a tier of execution of a module. It's
semantically a pair of (metadata, codesegment*). I intend to evolve
this class into the generic container of code for either 'full tiers',
'partial tiers', or 'lazy stubs'. So this commit renames the class
to something generic. CodeSegment would be a good candidate, but it's
already taken.

Depends on D213138

Comment 4

[Ryan Hunt [:rhunt]]

Attachment: Bug 1898153 - wasm: Make CodeSegment ref-counted instead of unique ownership. r?jseward

CS was already conceptually shared when used by lazy stubs, as each
lazy stub segment could be referenced by multiple lazy func exports
through an index. A future commit will make this more explicit, so
I'm changing this to be ref-counted in advance.

Depends on D213139

Comment 5

[Ryan Hunt [:rhunt]]

Attachment: Bug 1898153 - wasm: Have CodeBlock reference any kind of CodeSegment. r?jseward

I intend to replace LazyStubSegment with CodeBlock, by using the list of
CodeRange's on CodeBlock instead of having them live on CodeSegment. This
will allow us to turn CodeSegment into a truly immutable code allocation
that doesn't care about what's stored in it.

The first step is to have CodeBlock be generic about whether it holds
a ModuleSegment or a LazyStubSegment. Future steps will get rid of the
difference.

Depends on D213140

Comment 6

[Ryan Hunt [:rhunt]]

Attachment: Bug 1898153 - wasm: Create CodeBlock for tracking CodeRange's of new lazy stubs. r?jseward

This furthers the goal of removing LazyStubSegment, by creating a CodeBlock for
each new LazyStubSegment and putting the code ranges for each lazy stub in that
new code block. This duplicates the code ranges on LSS, but those will be removed
later.

Depends on D213141

Comment 7

[Ryan Hunt [:rhunt]]

Attachment: Bug 1898153 - wasm: Use CodeBlock for LazyFuncExport information, instead of LazyStubSegment. r?jseward

Now that we have a code block for every new chuck of lazy stubs we generate, we
can use that for finding a code range instead of LazyStubSegment.

Depends on D213142

Comment 8

[Ryan Hunt [:rhunt]]

Attachment: Bug 1898153 - wasm: Track CodeBlock in process global map, instead of CodeSegment. r?jseward

Now that we have CodeBlock as the authoritative source of code ranges, we
can use CodeBlock in the process global map instead of CodeSegments.

Depends on D213143

Comment 9

[Ryan Hunt [:rhunt]]

Attachment: Bug 1898153 - wasm: Merge ModuleSegment and LazyStubSegment. r?jseward

Merge ModuleSegment and LazyStubSegment into a unified CodeSegment class.

CodeSegment is now just a container of linked JIT code memory. It may either
be created from a MacroAssembler + LinkData, or else from just a byte capacity
that allows for it to accept dynamically generated code.

• Initialization logic is re-ordered so that sending code ranges to the profiler
  happens when initializing Code, not CodeSegment. This allows dropping the dependency
  on Metadata from CodeSegment.
• trapCode* is moved from ModuleSegment to Code. This removes the last special piece of
  metadata that lives on CodeSegment. This is safe because the trap code generated per
  tier is the same.

Depends on D213144

Comment 10

[Ryan Hunt [:rhunt]]

Attachment: Bug 1898153 - wasm: Flatten LazyStubTier into Code, and simplify. r?jseward

LazyStubTier has useful infrastructure needed for demand generating
new code, so this commit flattens it into Code so we can re-use it.

This unlocks an opportunity to simplify how commiting a tier2 works,
because all of the data is now in one place (Code). This commit
simplifies this logic too.

Depends on D213145

Comment 11

[Ryan Hunt [:rhunt]]

Attachment: Bug 1898153 - wasm: Rework initialization of Code and store tier blocks in vector. r?jseward

This commit reworks the creation of Code so that tier1 is provided at
init(), not the constructor. This lets us store tier1, and tier2 in the
Code vector of blocks, giving us a unified list of all the code blocks
inside of a code. The list of code segments is renamed to show that they
are only for lazy stubs, to avoid confusion.

Depends on D213146

Comment 12

[Ryan Hunt [:rhunt]]

Attachment: Bug 1898153 - wasm: Use ThreadSafeCodeBlockMap for PC-based metadata lookups. r?jseward

Take the ThreadSafeCodeBlockMap from WasmProcess.cpp, and generalize it so
that we can have more than one per-process. Then embed one in wasm::Code,
and use that for all PC-based metadata lookups we have.

This allows us to avoid having to iterate over all CodeBlocks (which requires
taking a lock), when we're looking up a piece of metadata.

Depends on D213147

Comment 13

[Ryan Hunt [:rhunt]]

Attachment: Bug 1898153 - wasm: Refactor CodeBlock::funcToCodeRange_ to be abstract. r?jseward

CodeBlock::funcToCodeRange_ only works because CodeBlock's for a tier of code are
'complete' and contain all funcs in the module. This won't be the case when we
support partial tiers, or move the imported function wrappers to their own block.

This commit reworks funcToCodeRange_ to be an abstract class that may or may not
be a dense vector. Right now only a dense vector is supported, but a sparse hash
map could be added in the future.

Depends on D213148

Comment 14

[Ryan Hunt [:rhunt]]

Attachment: Bug 1898153 - wasm: Refactor ModuleGenerator for supporting multiple code blocks. r?jseward

This commit reworks the logic in ModuleGenerator so that it's ready
to generate multiple code blocks. The existing code path for a
single 'complete tier' is made into a new method.

Depends on D213149

Comment 15

[Ryan Hunt [:rhunt]]

Attachment: Bug 1898153 - wasm: Generate all tier-independent stubs together in ModuleGenerator. r?jseward

Generate all of the stubs that don't depend on any tier-specific data in
one chunk at the beginning of the CodeBlock. The tier-specific stubs (entry
stubs) are generated separately at the end of the CodeBlock.

The purpose of this is to prepare ModuleGenerator for generating tier
-independent data into a separate CodeBlock from the rest of the module
code.

Depends on D213150

Comment 16

[Ryan Hunt [:rhunt]]

Attachment: Bug 1898153 - wasm: Generate tier-shared stubs in separate CodeBlock. r?jseward

TODO: document

Depends on D213151

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7f2ace29e124
wasm: Have only one LazyStubTier and put it on Code. r=jseward
https://hg.mozilla.org/integration/autoland/rev/1de85a4ce7ae
wasm: Merge MetadataTier into CodeTier. r=jseward
https://hg.mozilla.org/integration/autoland/rev/97276733ce7a
wasm: Rename CodeTier to CodeBlock. r=jseward
https://hg.mozilla.org/integration/autoland/rev/621c4254adf7
wasm: Make CodeSegment ref-counted instead of unique ownership. r=jseward
https://hg.mozilla.org/integration/autoland/rev/23f8fa8e529f
wasm: Have CodeBlock reference any kind of CodeSegment. r=jseward
https://hg.mozilla.org/integration/autoland/rev/7e13f23fb1c9
wasm: Create CodeBlock for tracking CodeRange's of new lazy stubs. r=jseward
https://hg.mozilla.org/integration/autoland/rev/ca74887894e6
wasm: Use CodeBlock for LazyFuncExport information, instead of LazyStubSegment. r=jseward
https://hg.mozilla.org/integration/autoland/rev/38d3fe022100
wasm: Track CodeBlock in process global map, instead of CodeSegment. r=jseward
https://hg.mozilla.org/integration/autoland/rev/78c1ed882b98
wasm: Merge ModuleSegment and LazyStubSegment. r=jseward
https://hg.mozilla.org/integration/autoland/rev/b438aa6a0a81
wasm: Flatten LazyStubTier into Code, and simplify. r=jseward
https://hg.mozilla.org/integration/autoland/rev/77d395f4c562
wasm: Rework initialization of Code and store tier blocks in vector. r=jseward
https://hg.mozilla.org/integration/autoland/rev/3b80e07c38bd
wasm: Use ThreadSafeCodeBlockMap for PC-based metadata lookups. r=jseward
https://hg.mozilla.org/integration/autoland/rev/3e72652d9496
wasm: Refactor CodeBlock::funcToCodeRange_ to be abstract. r=jseward
https://hg.mozilla.org/integration/autoland/rev/ff59e2597abb
wasm: Refactor ModuleGenerator for supporting multiple code blocks. r=jseward
https://hg.mozilla.org/integration/autoland/rev/0d18640a68ae
wasm: Generate all tier-independent stubs together in ModuleGenerator. r=jseward
https://hg.mozilla.org/integration/autoland/rev/ce76b2fd1fdd
wasm: Generate tier-shared stubs in separate CodeBlock. r=jseward
https://hg.mozilla.org/integration/autoland/rev/6b2961327cb5
apply code formatting via Lando

Comment 18

[Aron Cseh]

https://hg.mozilla.org/mozilla-central/rev/7f2ace29e124
https://hg.mozilla.org/mozilla-central/rev/1de85a4ce7ae
https://hg.mozilla.org/mozilla-central/rev/97276733ce7a
https://hg.mozilla.org/mozilla-central/rev/621c4254adf7
https://hg.mozilla.org/mozilla-central/rev/23f8fa8e529f
https://hg.mozilla.org/mozilla-central/rev/7e13f23fb1c9
https://hg.mozilla.org/mozilla-central/rev/ca74887894e6
https://hg.mozilla.org/mozilla-central/rev/38d3fe022100
https://hg.mozilla.org/mozilla-central/rev/78c1ed882b98
https://hg.mozilla.org/mozilla-central/rev/b438aa6a0a81
https://hg.mozilla.org/mozilla-central/rev/77d395f4c562
https://hg.mozilla.org/mozilla-central/rev/3b80e07c38bd
https://hg.mozilla.org/mozilla-central/rev/3e72652d9496
https://hg.mozilla.org/mozilla-central/rev/ff59e2597abb
https://hg.mozilla.org/mozilla-central/rev/0d18640a68ae
https://hg.mozilla.org/mozilla-central/rev/ce76b2fd1fdd
https://hg.mozilla.org/mozilla-central/rev/6b2961327cb5

Comment 19

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

This caused crashes with [@ js::wasm::CodeBlock::lookupFuncExport] and will be backed out in the next Nightly. Example: bp-dfde5488-c443-4b05-95eb-543630240701

Comment 20

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/d88ebdb41052
Backed out 17 changesets as reuqested by Aryx for causing wasm crashes.

Comment 21

[tszentpeteri]

Backed out for causing wasm crashes.

• Backout link

Comment 22

[Ryan Hunt [:rhunt]]

This looks like a race condition where:

1. Tiering is used
2. A wasm import function wrapper is implicitly exported through a funcref
3. The wrapper is called, creating an on-demand lazy function export
4. Tier-2 is finished compiling after this

We don't have any tests for this, and adding a test requires some new features in the harness to reliably trigger this. I added them locally and was able to reproduce this issue, and fix it.

https://treeherder.mozilla.org/jobs?repo=try&revision=6fb72a4314275e357cc608b6f34599e6b4e121a5

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/19c4b619cb0d
wasm: Have only one LazyStubTier and put it on Code. r=jseward
https://hg.mozilla.org/integration/autoland/rev/94b66174cb5f
wasm: Merge MetadataTier into CodeTier. r=jseward
https://hg.mozilla.org/integration/autoland/rev/bb38a0de22c1
wasm: Rename CodeTier to CodeBlock. r=jseward
https://hg.mozilla.org/integration/autoland/rev/5898ecc3f282
wasm: Make CodeSegment ref-counted instead of unique ownership. r=jseward
https://hg.mozilla.org/integration/autoland/rev/e54a28c570c6
wasm: Have CodeBlock reference any kind of CodeSegment. r=jseward
https://hg.mozilla.org/integration/autoland/rev/4e2ed3c03b44
wasm: Create CodeBlock for tracking CodeRange's of new lazy stubs. r=jseward
https://hg.mozilla.org/integration/autoland/rev/35cf07d63c5e
wasm: Use CodeBlock for LazyFuncExport information, instead of LazyStubSegment. r=jseward
https://hg.mozilla.org/integration/autoland/rev/cdf90f731b5c
wasm: Track CodeBlock in process global map, instead of CodeSegment. r=jseward
https://hg.mozilla.org/integration/autoland/rev/6f488caf8db3
wasm: Merge ModuleSegment and LazyStubSegment. r=jseward
https://hg.mozilla.org/integration/autoland/rev/3dcc782b8bfe
wasm: Flatten LazyStubTier into Code, and simplify. r=jseward
https://hg.mozilla.org/integration/autoland/rev/ecf9a3197b5b
wasm: Rework initialization of Code and store tier blocks in vector. r=jseward
https://hg.mozilla.org/integration/autoland/rev/ff8cad43732b
wasm: Use ThreadSafeCodeBlockMap for PC-based metadata lookups. r=jseward
https://hg.mozilla.org/integration/autoland/rev/87d34d2a98bb
wasm: Refactor CodeBlock::funcToCodeRange_ to be abstract. r=jseward
https://hg.mozilla.org/integration/autoland/rev/e36dc8935bc0
wasm: Refactor ModuleGenerator for supporting multiple code blocks. r=jseward
https://hg.mozilla.org/integration/autoland/rev/072f3c8e0d5c
wasm: Generate all tier-independent stubs together in ModuleGenerator. r=jseward
https://hg.mozilla.org/integration/autoland/rev/4704a988a3c4
wasm: Generate tier-shared stubs in separate CodeBlock. r=jseward
https://hg.mozilla.org/integration/autoland/rev/d6d2376f4bbd
apply code formatting via Lando

Comment 24

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/19c4b619cb0d
https://hg.mozilla.org/mozilla-central/rev/94b66174cb5f
https://hg.mozilla.org/mozilla-central/rev/bb38a0de22c1
https://hg.mozilla.org/mozilla-central/rev/5898ecc3f282
https://hg.mozilla.org/mozilla-central/rev/e54a28c570c6
https://hg.mozilla.org/mozilla-central/rev/4e2ed3c03b44
https://hg.mozilla.org/mozilla-central/rev/35cf07d63c5e
https://hg.mozilla.org/mozilla-central/rev/cdf90f731b5c
https://hg.mozilla.org/mozilla-central/rev/6f488caf8db3
https://hg.mozilla.org/mozilla-central/rev/3dcc782b8bfe
https://hg.mozilla.org/mozilla-central/rev/ecf9a3197b5b
https://hg.mozilla.org/mozilla-central/rev/ff8cad43732b
https://hg.mozilla.org/mozilla-central/rev/87d34d2a98bb
https://hg.mozilla.org/mozilla-central/rev/e36dc8935bc0
https://hg.mozilla.org/mozilla-central/rev/072f3c8e0d5c
https://hg.mozilla.org/mozilla-central/rev/4704a988a3c4
https://hg.mozilla.org/mozilla-central/rev/d6d2376f4bbd