[#2275] Refactor MemoryUsage to use LongAdder - #2276
Draft
mattrpav wants to merge 2 commits into
Draft
Conversation
JMH benchmark isolating the broker's topic send path: non-persistent
broker + NON_PERSISTENT BytesMessage, vm:// transport, async producer
send with unbounded window, producer flow control off, advisories/JMX/
scheduler/statistics/audit off, zero consumers. Parameters toggle a
shared topic vs a distinct topic per producer thread (distinctTopics)
and vm async dispatch vs direct dispatch on the calling thread
(vmAsync). Named *Benchmark so surefire skips it in CI.
Build (JDK 23+ needs proc=full for the JMH annotation processor):
mvn -pl activemq-unit-tests -am install -DskipTests -Dmaven.compiler.proc=full
Run (example: direct dispatch, both topic modes, thread sweep):
CP="activemq-unit-tests/target/test-classes:activemq-unit-tests/target/classes:$(mvn -q -pl activemq-unit-tests dependency:build-classpath -Dmdep.outputFile=/dev/stdout)"
java -cp "$CP" org.openjdk.jmh.Main TopicRawThroughputBenchmark \
-p numConsumers=0 -p useVirtualThread=false -p vmAsync=false \
-p distinctTopics=false,true -f 2 -wi 3 -w 2s -i 5 -r 2s \
-jvmArgs "-Xms4g -Xmx4g -XX:+AlwaysPreTouch"
On this commit (stock code) throughput DEGRADES as producer threads are
added, even with a topic per producer (e.g. ~5.2M msgs/sec at 1 thread
falling to ~1.1M at 22 threads on an 11-core machine). JMH stack
profiling attributes ~86% of lock wait time to the MemoryUsage
ReentrantReadWriteLock: every send takes exclusive write locks on a
usage hierarchy rooted in the broker-global SystemUsage, so destination
sharding cannot avoid the contention. The following commit removes it.
mattrpav
changed the title
mattrpav
marked this pull request as draft
August 2, 2026 11:59
mattrpav
changed the title
Replace the exclusive-lock byte counter in MemoryUsage with a striped
java.util.concurrent.atomic.LongAdder and make percentUsage volatile.
increaseUsage/decreaseUsage become lock-free adds; the existing
usageLock/setPercentUsage path (listener events, waitForSpace
signalling) is entered only when the rounded percentUsage actually
changes - at most ~100/percentUsageMinDelta locked updates per limit
traversal instead of one exclusive write lock (plus the parent chain's)
per message. isFull() becomes a volatile read. waitForSpace and all
public APIs are unchanged.
Rationale: profiling the previous commit's benchmark showed ~86% of
lock wait time in this class - Topic.send -> isFull (read lock),
Message.incrementReferenceCount -> increaseUsage (write lock) and
decrementReferenceCount -> decreaseUsage (write lock), each recursing
into the broker-global SystemUsage parent. That made usage accounting a
broker-wide serialization point that destination sharding cannot avoid.
The multi-producer degradation curve is eliminated; the shared-topic
and uncontended single-producer cases also improve (~+22% / ~+11%).
Allocation is unchanged (JMH gc profiler: ~480 vs ~527 B/op, GC count
~0 in both), so the gain is purely lock behaviour.
Semantics validation (all pass against this change):
ProducerFlowControlTest (7), ProducerFlowControlSendFailTest (8),
TopicProducerFlowControlTest (3), CompositeMessageCursorUsageTest (1),
QueueMemoryAndStoreUsageCleanupTest (1).
Known trade-off: between an add and the locked percent update there is
a sub-percentUsageMinDelta staleness window; rapid crossings may
coalesce listener events (pairs stay consistent and the locked update
recomputes from the live sum, so it self-corrects). StoreUsage and
TempUsage still use the base-class locked path and can be converted the
same way as a follow-up.
Fix setUsage() concurrent safety - delta set instead of LongAdder.reset()
LongAdder.reset() is documented as safe only when there are no
concurrent updates: a racing increaseUsage/decreaseUsage could be lost
outright (or, in narrow cell-sweep interleavings, applied out of
program order), permanently corrupting the accounting. Implement
setUsage(value) as a single delta add instead:
usage.add(value - usage.sum());
Every interleaving with a concurrent update now maps to a legal
linearization (the update is preserved, as if ordered after the set).
Also rename the parameter to avoid shadowing the field, and document
that setUsage - like the historical plain field assignment - does not
propagate an adjustment to the parent usage.
Tests: adds a sequential setUsage semantics test and a 200-round
concurrent drift-bound hammer (16 threads of balanced increase/decrease
pairs racing a mid-flight setUsage; final usage must stay within one
in-flight op per thread of the set value). Honest note: the hammer did
NOT empirically catch the reset() implementation in 200 rounds - a
same-thread increase/decrease pair usually lands in the same adder
cell, so both are wiped or both survive and the loss window is
vanishingly narrow. The fix is justified by LongAdder's documented
contract; the test remains as a regression guard on the delta
implementation's bound.
Validation: MemoryUsageConcurrencyTest (3) and MemoryUsageTest (5)
pass; ProducerFlowControlTest, ProducerFlowControlSendFailTest and
TopicProducerFlowControlTest (18 total) pass against the patched
client. setUsage has no production callers (test/config only), so this
is hardening, not a behavior change for the broker runtime.
Harden lock-free usage accounting - liveness soak, invariant docs
Untimed waitForSpace() has no polling fallback: it blocks on
waitForSpaceCondition and depends entirely on a lasting full ->
not-full transition reaching the locked setPercentUsage() path. With
lock-free accounting that liveness rests on an ordering invariant
(every usage.add() is unconditionally followed by maybeUpdatePercent(),
so the temporally last mutation of a lasting drop recomputes from the
complete sum and signals). This commit:
- documents the invariant loudly at both mutation sites in MemoryUsage
so a future edit cannot silently break it, and
- adds testUntimedWaitForSpaceLivenessSoak: 150 rounds parking untimed
waiters at exactly 100% usage, racing balanced increase/decrease
churn (usage never drops below full), then issuing the lasting
one-byte drop WHILE churn runs; after churn quiesces every waiter
must be released. Passes consistently.
GC evidence for the review record (send_08_threads, distinctTopics,
direct dispatch, JDK 25, -prof gc): per-op allocation is unchanged by
the LongAdder change - 467.5 +/- 36.4 B/op stock vs 440.0 +/- 0.2 B/op
patched. Absolute alloc rate rises only in proportion to throughput
(1.34M -> 5.69M ops/s in the profiled runs); gc.time was 17ms vs 19ms
over the measurement window. The change adds no per-operation
allocation and no meaningful GC pressure.
mattrpav
force-pushed
the
amq-gh-2275-usage-longadder
branch
from
August 2, 2026 21:57
ad3abff to
3ebd8d3
Compare
mattrpav
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.