perf: make IggyMessagesBatch::last_offset O(1)#2840
perf: make IggyMessagesBatch::last_offset O(1)#2840cijiugechu wants to merge 3 commits intoapache:masterfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2840 +/- ##
============================================
- Coverage 67.69% 67.65% -0.04%
Complexity 739 739
============================================
Files 1031 1031
Lines 83912 83918 +6
Branches 60706 60722 +16
============================================
- Hits 56802 56778 -24
- Misses 24760 24779 +19
- Partials 2350 2361 +11
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
Hi, thanks for the contribution. I am pretty sure a simpler solution to that problem would be to provide the The clue is those two lines mov rdi, qword ptr [rbx + 8]
mov rbx, qword ptr [rdi + 8*rax - 8] ;Load the ptr + (len - 1) |
The |
|
Maybe we should consider implementing more efficient Iterator methods to override the default behavior, so we don’t end up relying on std fallback implementation... |
Right, so maybe a idea would be to implement |
I gave it a try, and implementing |
|
I got |
Which issue does this PR close?
None.
Rationale
Avoid unnecessary O(n) lookup in
IggyMessagesBatch::last_offset()to improve throughput.Bench (single run each; Apple M4, macOS 15.7.4; pinned-producer/TCP; 8 producers, 8 streams; 1000B msgs; 1000 msgs/batch; total data 5GB):
Results:
Throughput delta: ~88.75Mb/s
master:
PR:
What changed?
IggyMessagesBatch::last_offset()previously usediter().last(), which walk the whole batch and made the call O(n) even though the last element is known. In producer hot paths with largemessages_per_batch, that extra scan is on the critical path.The method now reads the last message via indexed
get(count - 1)(O(1)), with an iterator fallback only if the indexed lookup fails.Local Execution
AI Usage
None.