Repository navigation
Atomics followup - #237
Atomics followup#237
Conversation
- ByteBufferMemory.waitOn reads the expected value with atomicReadInt/Long, so the check is ordered with atomic stores that now lock the containing 8-byte word rather than the exact address (matches ByteArrayMemory and the Redline memories). - Non-shared memories skip locking again and don't allocate the wait state map: they can't be accessed by more than one wasm thread, and this is what the Redline memories already do. - ByteArrayMemory only goes lock-free when short volatile get/set are also supported, so short atomic reads/writes can't end up locked while everything else is lock-free. The per-size volatile flags are implied by the full atomic checks and are dropped.
Locked atomics used the wait state map as their lock table, so every
atomic boxed the word address, went through ConcurrentHashMap
computeIfAbsent and left an entry behind. That is every atomic on a
shared ByteBufferMemory (the default memory) and, on JDK 22+, on a shared
ByteArrayMemory.
Atomics now lock one of 256 stripes picked from the 8-byte word with
Fibonacci hashing, so per-thread data at power-of-two strides doesn't
pile onto one stripe. The wait state map only holds wait queues. Lock
order is still wait state -> stripe (waitOn reads the value atomically)
and atomics never take a wait state, so this can't deadlock.
atomicAddInt throughput on a shared memory, 4 threads, ops/us
(map -> stripes, local JMH, laptop, noisy):
own word own 1024 words
JDK 21 ByteBuffer 76 -> 124 39 -> 133
JDK 25 ByteBuffer 54 -> 143 45 -> 129
JDK 25 ByteArray 58 -> 145 58 -> 145
Single-threaded and all-threads-on-one-word are unchanged within noise.
|
@andreaTP will have a look by eod. |
There was a problem hiding this comment.
I really like the new WordLocks solution.
My only concern is silently dropping atomicity on unshared memories. With the striped implementation it shouldn't cost much, and even single-threaded it can be very useful, for instance to swap in a new configuration from the host. The spec allows it: "All atomic memory accesses can be performed on both shared and unshared linear memories." It doesn't cover visibility to the host, though it does anticipate host interaction with unshared memory (notify on unshared memory has zero waiters "unless the host has created such a waiter"). Either way, silently dropping atomicity doesn't seem like a good choice to me.
Atomics are valid on non-shared memories and the host can touch them from another thread, so don't skip locking there (as bytecodealliance#233 intended). Every memory now owns its word locks; the wait state map stays shared-only. Fallback atomics on a non-shared memory (ByteBufferMemory on any JDK, ByteArrayMemory on JDK 22+) now cost an uncontended lock, about what a shared memory pays: single-threaded atomicAddInt went from ~2 ns (no-op lock, elided by C2) to ~19 ns on a laptop. ByteArrayMemory on JDK 11-21 keeps using hardware atomics. New tests: concurrent adds on non-shared memories (fail before this change on ByteBufferMemory, and on ByteArrayMemory on JDK 22+), and wait/notify mixed with atomics on the same 8-byte word.
Last commit should have restored that, to be clear, I get your reasoning and the motivations, at the same time, Threads is still a Wasm proposal, so we might expect changes to the spec, I'd favor having a consistent implementation for "shared" memories, but I'm not (yet?) aiming for feature parity of non-shared memories. |
2 participants
@andreas-karlsson I went after some findings after your last PR, I'll wait for your feedback here 🙏