You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
rmw_subscription_data.cpp allocates four times the payload plus 64 KiB for every message it deserialises, then copies the payload into it:
// FastCDR needs extra space for internal operations during deserialization// Allocate a larger buffer and copy the payload data// TODO(wjwwood): Use actual serialized message size instead of conservative estimatesize_t buffer_size = payload_data.size() * 4 + 65536; // 4x + 64KB safety margin
The TODO asks for the actual size. Implementing it as written costs 30-40% of round-trip latency at 1 MiB. This holds for both C++ and Python nodes. No test covers it.
Important
The buffer is in the subscription path, yet which endpoint pays depends on allocation history rather than on the code. Right-size this and profile the subscriber. You get a clean process and a slower system. In the C++ pair measured below, the sender pays and the receiver stays clean. This is the finding a reader cannot derive from the diff, so it comes first.
Two separate observations
1. This path allocates and copies where the released branches do not.
kilted, jazzy and humble wrap the payload in place:
So did rolling, before e95c62d (ros2/rmw_zenoh#930). That commit introduced the allocate-and-copy: 4 MiB resident per in-flight 1 MiB message, plus a full-payload copy per message, where previously there was neither. The commit is a large feature change. The buffer was not its focus, so its cost may have gone unnoticed.
2. The over-allocation is, by accident, protecting latency.
glibc sets M_TRIM_THRESHOLD to twice M_MMAP_THRESHOLD. It raises the latter toward the size of the large blocks a process frees. The oversized block lifts the trim threshold clear of the node's working set. A right-sized block does not raise the threshold. glibc returns the heap with brk between messages, and the next message re-faults it.
The allocate call is the only place that uses buffer_size. Both builds construct FastBuffer over payload_data.size(). FastCDR never sees the margin, so it behaves identically in both cases. The only difference between the two builds measured below is the integer passed to the allocator.
Measured
Both builds come from one source tree. They differ only in that line. Each repetition interleaves both arms within a single benchmark run. The run rotates the arm order, so the router and the machine state stay the same for both arms.
client
arm
median p50
vs stock
reps
Python
stock
3279 us
—
12
Python
right-sized
4287 us
+30.5%
12/12 slower
Python
right-sized + MALLOC_* pinned
3397 us
+3.6%
recovered
C++
stock
2313 us
—
8
C++
right-sized
3215 us
+36.8%
8/8 slower
C++
right-sized + MALLOC_* pinned
2302 us
-2.0%
fully recovered
Ranges are disjoint in both clients: the stock arm's slowest run is faster than the right-sized arm's fastest.
This is a per-message cost, not a queueing effect. At 10 Hz there is 100 ms of slack per message. The C++ gap is still +42.0% (6/6), against +40.9% at 200 Hz. Absolute latencies do not depend on the rate.
A 64 B negative control moves -0.6% (range -0.8% to +0.9%). The effect is absent where the mechanism says it must be.
The mechanism, in syscalls
Counted on the process that pays, same 15 s window:
stock
right-sized
page faults
1,025
1,440,993
brk
0
8,997
mmap / munmap
0 / 0
0 / 0
mmap is zero in both arms. Either way, glibc serves the buffer from the heap, so the over-allocating arm never pays a larger fault cost for its larger block. The thresholds decide trimming, not heap-versus-mmap. That is 480 faults per message, against 256 pages per MiB.
The part that is not derivable from the code
Which process pays is not predictable. The buffer is in the subscription path, so the receiver looks like the obvious victim. Measured:
client
receiver
sender
Python
pays — brk 17 → 5,889, faults 6,112 → 950,249
—
C++
flat — cycles 13.31 G vs 13.29 G, instructions 3.84 G vs 3.78 G
pays — the table above
Both endpoints run the same code. Both link the same library. They differ only in allocation history. That history decides where each process's dynamic threshold settles. So profiling the receiver alone can show a clean process while the node pair still runs 40% slower. That is what happened here, and it cost several hours of investigation.
It reproduces without ROS
Twenty lines of C, getrusage only:
for (i=0; i<iters; i++) {
for (j=0; j<live; j++) { buf[j] =malloc(sz); memset(buf[j], 1, sz); }
for (j=0; j<live; j++) free(buf[j]);
}
glibc
2.31
2.35
2.36
2.39
2.42
minor faults per iteration
992
992
992
992
992
with mallopt(M_TRIM_THRESHOLD, 64<<20)
1028
0
0
0
0
The trigger is two or more payload-sized buffers live at once, not a particular payload size. The threshold tracks one block, while free-space-at-top is N blocks. Testing confirms this at 256 KiB, 1 MiB and 4 MiB.
Suggested handling
option
note
Restore the in-place FastBuffer, as the released branches do
removes the allocation and the copy, and the margin protects nothing — see below
Right-size and set the allocator policy together
mallopt(M_TRIM_THRESHOLD, ...) at init, or document the environment variables. Note it sets process-global policy from inside a plugin, and does nothing on glibc 2.31
Keep the margin, and record why
the 4x currently does two jobs and only one is written down
Whichever option you choose, measure 1 MiB round-trip latency before and after, on both endpoints. Nothing in CI covers this today.
On the margin the comment justifies
The comment says FastCDR needs extra space for internal operations during deserialisation. It does not receive any:
buffer_size occurs once in the file, in the allocate call. The code constructs FastBuffer over payload_data.size(), not over buffer_size. So FastCDR cannot address the margin. FastBuffer also cannot grow a buffer it does not own. On overrun, it raises NotEnoughMemoryException instead of writing past the buffer. Removing the margin entirely ran 5,000 messages per repetition across every measurement above, with no deserialisation failure.
Two observations on how this may have arisen, offered as inference rather than as confirmed history:
The bound has read payload_data.size() in every revision of this code. That includes the code before e708d5d, three months before e95c62d. Its comment reads as a correction: "Use actual payload size, not allocated buffer size." A FastBuffer that expected 4x the real data would read past the message into uninitialised memory. The fix removed that risk. Nobody revisited the allocation afterward.
The publisher path in the same package sizes its buffer from get_estimated_serialized_size(). The 4x guess appears exactly once in the tree, on the side that needs it least. Serialisation must fit an output of unknown length. Deserialisation already has the exact length before it starts.
This margin may be load-bearing for some type or encoding not tested here. If so, the comment should say which one. The current comment describes an intent the code does not implement.
Independent corroboration in a second codebase
An unrelated Rust ROS 2 middleware also right-sizes its deserialisation buffer, and its authors independently hit the same defect. ZettaScaleLabs/hiroz#349 reports the defect, measured. ZettaScaleLabs/hiroz#350 is the fix: a mallopt call at initialisation. That pull request is open, and its CI passes as of this writing; it is not yet merged.
The codebase differs and the language differs, but the glibc interaction matches, the regression size matches (30-40% at 1 MiB), and the fix shape matches. This is corroboration: the defect is a property of the allocator interaction, not of one code path.
What this issue does not claim
Not that this affects released distros. They use a different code path. This issue does not measure them.
Not that mallopt is a general fix. It does nothing on glibc 2.31.
Not that the current code is slow. As shipped, it is the fast configuration. The concern is the change the TODO invites.
rmw_subscription_data.cppallocates four times the payload plus 64 KiB for every message it deserialises, then copies the payload into it:The TODO asks for the actual size. Implementing it as written costs 30-40% of round-trip latency at 1 MiB. This holds for both C++ and Python nodes. No test covers it.
Important
The buffer is in the subscription path, yet which endpoint pays depends on allocation history rather than on the code. Right-size this and profile the subscriber. You get a clean process and a slower system. In the C++ pair measured below, the sender pays and the receiver stays clean. This is the finding a reader cannot derive from the diff, so it comes first.
Two separate observations
1. This path allocates and copies where the released branches do not.
kilted,jazzyandhumblewrap the payload in place:So did
rolling, beforee95c62d(ros2/rmw_zenoh#930). That commit introduced the allocate-and-copy: 4 MiB resident per in-flight 1 MiB message, plus a full-payload copy per message, where previously there was neither. The commit is a large feature change. The buffer was not its focus, so its cost may have gone unnoticed.2. The over-allocation is, by accident, protecting latency.
glibc sets
M_TRIM_THRESHOLDto twiceM_MMAP_THRESHOLD. It raises the latter toward the size of the large blocks a process frees. The oversized block lifts the trim threshold clear of the node's working set. A right-sized block does not raise the threshold. glibc returns the heap withbrkbetween messages, and the next message re-faults it.The
allocatecall is the only place that usesbuffer_size. Both builds constructFastBufferoverpayload_data.size(). FastCDR never sees the margin, so it behaves identically in both cases. The only difference between the two builds measured below is the integer passed to the allocator.Measured
Both builds come from one source tree. They differ only in that line. Each repetition interleaves both arms within a single benchmark run. The run rotates the arm order, so the router and the machine state stay the same for both arms.
MALLOC_*pinnedMALLOC_*pinnedRanges are disjoint in both clients: the stock arm's slowest run is faster than the right-sized arm's fastest.
This is a per-message cost, not a queueing effect. At 10 Hz there is 100 ms of slack per message. The C++ gap is still +42.0% (6/6), against +40.9% at 200 Hz. Absolute latencies do not depend on the rate.
A 64 B negative control moves -0.6% (range -0.8% to +0.9%). The effect is absent where the mechanism says it must be.
The mechanism, in syscalls
Counted on the process that pays, same 15 s window:
brkmmap/munmapmmapis zero in both arms. Either way, glibc serves the buffer from the heap, so the over-allocating arm never pays a larger fault cost for its larger block. The thresholds decide trimming, not heap-versus-mmap. That is 480 faults per message, against 256 pages per MiB.The part that is not derivable from the code
Which process pays is not predictable. The buffer is in the subscription path, so the receiver looks like the obvious victim. Measured:
brk17 → 5,889, faults 6,112 → 950,249Both endpoints run the same code. Both link the same library. They differ only in allocation history. That history decides where each process's dynamic threshold settles. So profiling the receiver alone can show a clean process while the node pair still runs 40% slower. That is what happened here, and it cost several hours of investigation.
It reproduces without ROS
Twenty lines of C,
getrusageonly:mallopt(M_TRIM_THRESHOLD, 64<<20)The trigger is two or more payload-sized buffers live at once, not a particular payload size. The threshold tracks one block, while free-space-at-top is N blocks. Testing confirms this at 256 KiB, 1 MiB and 4 MiB.
Suggested handling
FastBuffer, as the released branches domallopt(M_TRIM_THRESHOLD, ...)at init, or document the environment variables. Note it sets process-global policy from inside a plugin, and does nothing on glibc 2.31Whichever option you choose, measure 1 MiB round-trip latency before and after, on both endpoints. Nothing in CI covers this today.
On the margin the comment justifies
The comment says FastCDR needs extra space for internal operations during deserialisation. It does not receive any:
buffer_sizeoccurs once in the file, in theallocatecall. The code constructsFastBufferoverpayload_data.size(), not overbuffer_size. So FastCDR cannot address the margin. FastBuffer also cannot grow a buffer it does not own. On overrun, it raisesNotEnoughMemoryExceptioninstead of writing past the buffer. Removing the margin entirely ran 5,000 messages per repetition across every measurement above, with no deserialisation failure.Two observations on how this may have arisen, offered as inference rather than as confirmed history:
payload_data.size()in every revision of this code. That includes the code beforee708d5d, three months beforee95c62d. Its comment reads as a correction: "Use actual payload size, not allocated buffer size." AFastBufferthat expected 4x the real data would read past the message into uninitialised memory. The fix removed that risk. Nobody revisited the allocation afterward.get_estimated_serialized_size(). The 4x guess appears exactly once in the tree, on the side that needs it least. Serialisation must fit an output of unknown length. Deserialisation already has the exact length before it starts.This margin may be load-bearing for some type or encoding not tested here. If so, the comment should say which one. The current comment describes an intent the code does not implement.
Independent corroboration in a second codebase
An unrelated Rust ROS 2 middleware also right-sizes its deserialisation buffer, and its authors independently hit the same defect. ZettaScaleLabs/hiroz#349 reports the defect, measured. ZettaScaleLabs/hiroz#350 is the fix: a
malloptcall at initialisation. That pull request is open, and its CI passes as of this writing; it is not yet merged.The codebase differs and the language differs, but the glibc interaction matches, the regression size matches (30-40% at 1 MiB), and the fix shape matches. This is corroboration: the defect is a property of the allocator interaction, not of one code path.
What this issue does not claim
malloptis a general fix. It does nothing on glibc 2.31.