mwashburn2 opened a new pull request, #2671:
URL: https://github.com/apache/activemq/pull/2671
## Problem
`SystemUsage.getMemoryUsage()` is one `MemoryUsage` instance shared by every
destination (`BaseDestination` uses it directly by default). Every message's
reference count transitions (`Message.incrementReferenceCount` /
`decrementReferenceCount`) take its `ReentrantReadWriteLock` **write** lock via
`increaseUsage` / `decreaseUsage`. Every `Topic.send` / `Queue.send` takes the
**read** lock via `isFull()`, unconditionally and before the
producer-flow-control check. Cursor `hasSpace()` also takes it via
`getPercentUsage()`.
On a busy many-core broker this produces a lock convoy. Throughput is capped
by park/unpark hand-offs on that one lock, even though CPU and network have
plenty of headroom.
### Production evidence (5.18.x, 64 cores, ~600 transport threads, mostly
non-persistent topics)
Five thread dumps were taken about 30s apart. The global `MemoryUsage` RRWL
was the dominant contention point in every one:
| Dump | Threads parked on global usage RRWL | Threads parked on queue
cursor RRWLs (holders blocked on the global lock) |
|---|---|---|
| 1 | 49 | 15 |
| 2 | 34 | 2 |
| 3 | 26 | 45 |
| 4 | 60 | 26 |
| 5 | 58 | 8 |
Breakdown from one dump, by operation on the lock:
| Operation | Waiters |
|---|---|
| `isFull` ← `Topic.send` | 38 |
| `isFull` ← `Queue.send` | 9 |
| `decreaseUsage` ← `decrementReferenceCount` | 7 |
| `increaseUsage` ← `incrementReferenceCount` | 3 |
| `getPercentUsage` ← `isParentFull` | 3 |
No thread was ever RUNNABLE inside `setPercentUsage` or `fireEvent`. The
cost is purely lock hand-off, not work done under the lock.
Waiting threads often hold a `Topic` monitor or a queue cursor lock. Other
transport threads then block behind them and stop reading their sockets, and
producers stall in `send()` while broker CPU sits mostly idle. The enqueue rate
stayed flat at a steady ceiling regardless of offered load.
The memory limit was never close to being hit, so nobody was actually
waiting for space.
## Change
- `MemoryUsage.usage` is an `AtomicLong`. `increaseUsage` / `decreaseUsage`
/ `getUsage` no longer lock.
- `Usage.percentUsage`, `limiter`, `percentUsageMinDelta`, `usagePortion`
and `DefaultUsageCapacity.limit` are `volatile`. `getPercentUsage()`,
`getLimit()`, `getUsagePortion()`, `getPercentUsageMinDelta()` and
`MemoryUsage.isFull()` / `isFull(int)` are lock-free.
- `percentUsage` is still **written only under the write lock**, and only
when the computed value actually changes. `setPercentUsage` → `fireEvent` and
the `waitForSpace` condition signalling therefore keep their existing
semantics. Most inc/dec calls do not move the percentage, so the lock is rarely
taken.
- `waitForSpace()` / `waitForSpace(long)` return immediately when not full.
They only lock and await when full, rechecking under the lock.
- `percentUsage` is recomputed on `setUsage`, `onLimitChange` and
`setPercentUsageMinDelta`, so it cannot go stale.
- `StoreUsage`, `TempUsage`, `JobSchedulerUsage` and `PercentLimitUsage` are
unchanged. They still use the locked base paths.
### Why it is safe
- Updaters publish the new usage with `addAndGet` (volatile write), then
compare the freshly computed percent to the volatile `percentUsage`.
- Whoever changes `percentUsage` does so under the write lock and loops (`do
{ v = calc(); setPercentUsage(v); } while (v != calc())`). The last updater to
publish therefore always leaves `percentUsage` consistent with the current
usage, and no stale value can survive.
- Readers (`isFull`, `getPercentUsage`) may see a value one update behind.
That was already true for callers using the result after releasing the read
lock.
- `LongAdder` was considered and rejected because `sum()` is not an exact
snapshot.
## Microbenchmark
The benchmark uses 16 destination `MemoryUsage`s sharing one parent (limit
123 GB, about 48% used). Each op is `increaseUsage(1024); isFull();
getPercentUsage(); parent.isFull(); decreaseUsage(1024)`, which mirrors the
per-message broker pattern. Results from JDK 25 on 64 cores:
| Threads | Before (ops/s) | After (ops/s) | Speedup |
|---|---|---|---|
| 1 | 19.4M | 93.8M | 4.8x |
| 8 | 6.3M | 34.5M | 5.5x |
| 32 | 5.5M | 14.3M | 2.6x |
| 64 | 5.8M | 24.3M | 4.2x |
## Tests
- New `MemoryUsageLockFreeTest` (9 tests) covers:
- concurrent inc/dec exactness
- percent consistency after concurrent updates
- listener events on threshold crossing
- `waitForSpace` blocking and releasing, with and without timeout
- parent propagation
- limit and min-delta changes
- `setUsage`
- These existing tests pass:
| Module | Tests |
|---|---|
| `activemq-client` | `MemoryUsageTest`, `MemoryUsageConcurrencyTest` |
| `activemq-unit-tests` | `MemoryUsageBlockResumeTest`,
`UsageBlockedDispatchTest`, `MemoryUsageCleanupTest`, `MemoryLimitTest`,
`CompositeMessageCursorUsageTest`, `ProducerFlowControlTest`,
`ProducerFlowControlSendFailTest`, `TopicProducerFlowControlTest`,
`StoreUsageTest`, `StoreUsageLimitsTest`,
`QueueMemoryAndStoreUsageCleanupTest`, `QueueOptimizedDispatchExceptionTest` |
Opening as a draft for feedback on the approach before a wider test run.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information, visit: https://activemq.apache.org/contact
