1667984 - Clean up locking and documentation around lazy stubs and the jump table

Status: RESOLVED FIXED

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

Description

[Lars T Hansen [:lth]]

A Module object is shared in several ways: among instances made from the module and between the executing thread and the background compilation threads. Importantly, the instances may be on different worker threads, so there are two dimensions of multithreading here.

A Module holds onto a Code, and the Code has one or two CodeTier objects. Each CodeTier has a LazyStubTier containing information about lazily generated stubs code for that tier's code. The CodeTier holds onto its LazyStubTier via an ExclusiveData lock. Code that generates stubs takes the lock for at least one, sometimes both lazy stubs tiers, in a complicated protocol. But the Code also has a JumpTables object. This has jump tables for wasm-to-wasm calls and for jit-to-wasm calls. The JumpTables object does not have a lock and is sometimes read and written racily (on purpose) via the setJitEntry and setTieringEntry methods on the Code. However, this mechanism is quite brittle and its correctness is far from obvious, and it is poorly documented, and may predate the days when we contemplated multithreaded wasm (multiple workers). Cleanup is required.

Some observations:

• finishTier2 locks both tiers together but the calls to setJitEntry are outside the scope of the locks
• EnsureEntryStubs takes one lock (for the best tier) and may take the other (if tiering was done concurrently), though could probably just take both even if it need only take the tier2 lock
• EnsureEntryStubs, unlike finishTier2, calls setJitEntry while its locks are held
• getExportedFunction needs to initialize a jump table entry but not if a module on a different thread has initialized it already, and it does this without taking any locks and calls setJitEntryIfNotNull
• There's code in the CodeTier that needs to lock the stubs table while not manipulating the jump table at all
• If we were to add a lock to the jump table then there are some paths that might be hot that would need to acquire that lock very frequently; this needs investigation
• If a jump table lock were to be added it would have lower priority than the lazy stubs tier locks

Comment 1

[Lars T Hansen [:lth]]

Attachment: Bug 1667984 - Document JumpTable, the stubs system, and tiering. r?rhunt

Comment 2

[Lars T Hansen [:lth]]

I added a big block on tiering logic and moved some other things around; I'm wondering if you'd mind sliding your eyes over this again to make sure it's coherent?

Comment 3

[Lars T Hansen [:lth]]

Scratch that, I want to do one more thing before I bother you with the patch.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6c026ef51eba
Document JumpTable, the stubs system, and tiering. r=rhunt DONTBUILD

Comment 5

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/6c026ef51eba