IGNITE-28304 Improve contention on waiters in lock manager#7970
IGNITE-28304 Improve contention on waiters in lock manager#7970Cyrill wants to merge 4 commits intoapache:mainfrom
Conversation
| expected[w.lockMode.ordinal()]++; | ||
| } | ||
| } | ||
| assert Arrays.equals(expected, lockModeHeldCount) |
There was a problem hiding this comment.
As I remember, we didn't want to use assert where it's not necessary, is it still so?
| for (WaiterImpl w : waiters.values()) { | ||
| if (w.lockMode != null) { | ||
| expected[w.lockMode.ordinal()]++; | ||
| } | ||
| } |
There was a problem hiding this comment.
Are you sure we want to check this every time?
Did you run benchmarks?
There was a problem hiding this comment.
Benchmarks (LockManagerBenchmark, 1 fork, default JMH settings):
| bench | main | this PR | Δ |
|---|---|---|---|
| contendedSharedKeyS (16 threads, shared key, S-mode) | 6.100 ± 0.271 µs/op | 4.809 ± 0.362 µs/op | −21% |
| lockCommit (1 thread, 200 unique X keys) | 22.757 µs/op | 19.022 µs/op | −16% |
|
|
||
| // Fast path: no incompatible holders — grant immediately. | ||
| // Conservative for upgrades: the waiter's own mode is in the counts, causing a fallthrough to the slow path. | ||
| if (!hasAnyIncompatibleHolder(intendedLockMode)) { |
There was a problem hiding this comment.
is it correct that the main idea is adding fast path for isWaiterReadyToNotify to reduce the time which tread spends inside synchronized?
There was a problem hiding this comment.
yes. More precisely: replace O(N) iteration with O(1) inside the synchronized block
| track(waiter.txId, this); | ||
| } | ||
|
|
||
| assertLockModeHeldCount(); |
There was a problem hiding this comment.
release flow seems to stay uncovered by this fix. Meanwhile, there is also a usage of heavy method under lock: unlockCompatibleWaiters
There was a problem hiding this comment.
Checked with a dedicated mixed X/S bench (releaseWakeupMixed: 1 X-writer + 15 S-readers on a shared key, NO_OP policy so S-readers actually queue behind the X-holder). In this setup every X release triggers unlockCompatibleWaiters over a batch of pending S waiters
| bench | main | this PR | Δ |
|---|---|---|---|
| releaseWakeupMixed | 4.169 ± 0.691 µs/op | 3.354 ± 0.295 µs/op | −20% |
| contendedSharedKeyS (16-thread S-only, acquire-side fast path) | 6.100 ± 0.271 µs/op | 4.809 ± 0.362 µs/op | −21% |
| lockCommit (1-thread, 200 unique X keys — sanity) | 22.757 µs/op | 19.022 µs/op | −16% |
https://issues.apache.org/jira/browse/IGNITE-28304