iobuf: fix operator<=> and operator== for string_view comparisons by travisdowns · Pull Request #29461 · redpanda-data/redpanda

travisdowns · 2026-01-29T16:21:48Z

Summary

Fix bug in iobuf::operator<=>(string_view) where inverted stop_iteration logic caused incorrect results for multi-fragment iobufs
Simplify iobuf::operator==(string_view) to delegate to the fixed operator<=>
Add benchmarks for iobuf vs string_view comparisons

Benchmark results

operator<=>:

Test	Instructions	Runtime
sv_cmp_0000	49 -> 36 (-27%)	2.28ns -> 2.16ns (-5%)
sv_cmp_0001	87 -> 64 (-26%)	3.61ns -> 3.17ns (-12%)
sv_cmp_1024 (different)	121 -> 60 (-50%)	4.98ns -> 3.44ns (-31%)
sv_cmp_1024 (same)	256 -> 214 (-16%)	10.37ns -> 9.41ns (-9%)

operator==:

Test	Instructions	Runtime
sv_eq_0000	38 -> 30 (-21%)	2.30ns -> 1.95ns (-15%)
sv_eq_0001	83 -> 58 (-30%)	3.61ns -> 2.96ns (-18%)
sv_eq_1024 (different)	80 -> 57 (-29%)	3.70ns -> 3.25ns (-12%)
sv_eq_1024 (same)	247 -> 209 (-15%)	10.50ns -> 9.46ns (-10%)

Test plan

Unit tests added for multi-fragment iobuf <=> string_view comparisons
Benchmarks added and show consistent improvements

Backports Required

Release Notes

none

The previous implementation had inverted stop_iteration logic - it stopped when the comparison was equal and continued when not equal, causing incorrect results when the first fragment comparison determined the ordering. Replaced iterator_consumer pattern with a simpler for-each loop over fragments. Added comprehensive tests covering multi-fragment comparisons. Benchmark results (instructions, runtime): - sv_cmp_0000: 49 -> 36 inst (-27%), 2.28ns -> 2.16ns (-5%) - sv_cmp_0001: 87 -> 64 inst (-26%), 3.61ns -> 3.17ns (-12%) - sv_cmp_1024 (different): 121 -> 60 inst (-50%), 4.98ns -> 3.44ns (-31%) - sv_cmp_1024 (same): 256 -> 214 inst (-16%), 10.37ns -> 9.41ns (-9%) Runtime improvements are consistent with instruction count reductions.

Simplify operator== by delegating to the fixed operator<=> implementation. This reduces code duplication and ensures consistent comparison behavior. Benchmark results (instructions, runtime): - sv_eq_0000: 38 -> 30 inst (-21%), 2.30ns -> 1.95ns (-15%) - sv_eq_0001: 83 -> 58 inst (-30%), 3.61ns -> 2.96ns (-18%) - sv_eq_1024 (different): 80 -> 57 inst (-29%), 3.70ns -> 3.25ns (-12%) - sv_eq_1024 (same): 247 -> 209 inst (-15%), 10.50ns -> 9.46ns (-10%) Runtime improvements are consistent with instruction count reductions.

Copilot

Pull request overview

This PR fixes a bug in iobuf::operator<=>(string_view) where inverted stop iteration logic caused incorrect comparison results for multi-fragment iobufs. The fix simplifies the implementation to use a clearer loop-based approach and delegates operator== to the corrected spaceship operator for consistency.

Changes:

Fixed operator<=> logic by replacing the iterator_consumer pattern with a simpler fragment-by-fragment loop
Simplified operator== to delegate to the fixed operator<=>
Added comprehensive unit tests for multi-fragment iobuf comparisons with string_view
Added benchmarks to measure performance of iobuf vs string_view comparisons

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/v/bytes/iobuf.cc	Fixed `operator<=>` implementation and simplified `operator==` to use the corrected spaceship operator
src/v/bytes/tests/iobuf_tests.cc	Added unit tests for multi-fragment iobuf spaceship operator comparisons with string_view
src/v/bytes/tests/iobuf_bench.cc	Added benchmarks for iobuf vs string_view equality and comparison operations

src/v/bytes/tests/iobuf_bench.cc

travisdowns · 2026-01-29T16:23:06Z

@rockwotj - not sure about backport here, I think you added the initial (maybe only) use case?

rockwotj · 2026-01-29T16:31:19Z

Needs a backport to 25.3.x

rockwotj

very embarrassing there are so many bugs here, need some more fuzzing here

travisdowns · 2026-01-29T17:52:20Z

need some more fuzzing here

This bug was actually turned up by fuzzing, as I had added more coverage to the fuzzer. That's in another PR.

vbotbuildovich · 2026-01-29T18:49:55Z

Retry command for Build#79846

please wait until all jobs are finished before running the slash command

/ci-repeat 1
skip-redpanda-build
skip-units
skip-rebase
tests/rptest/tests/write_caching_fi_e2e_test.py::WriteCachingFailureInjectionE2ETest.test_crash_all@{"use_transactions":false}

vbotbuildovich · 2026-01-29T19:02:25Z

CI test results

test results on build#79846

test_class	test_method	test_arguments	test_kind	job_url	test_status	passed	reason	test_history
WriteCachingFailureInjectionE2ETest	test_crash_all	{"use_transactions": false}	integration	https://buildkite.com/redpanda/redpanda/builds/79846#019c0ae7-aad5-4f7c-8e7f-5960189b3c0e	FLAKY	22/31	Test FAILS after retries.Significant increase in flaky rate(baseline=0.0841, p0=0.0027, reject_threshold=0.0100)	https://redpanda.metabaseapp.com/dashboard/87-tests?tab=142-dt-individual-test-history&test_class=WriteCachingFailureInjectionE2ETest&test_method=test_crash_all
TxAtomicProduceConsumeTest	test_basic_tx_consumer_transform_produce	{"with_failures": true}	integration	https://buildkite.com/redpanda/redpanda/builds/79846#019c0ae7-aad1-4c77-a4db-4727e3242904	FLAKY	10/11	Test PASSES after retries.No significant increase in flaky rate(baseline=0.0050, p0=1.0000, reject_threshold=0.0100. adj_baseline=0.1000, p1=0.3487, trust_threshold=0.5000)	https://redpanda.metabaseapp.com/dashboard/87-tests?tab=142-dt-individual-test-history&test_class=TxAtomicProduceConsumeTest&test_method=test_basic_tx_consumer_transform_produce
src/v/storage/tests/storage_e2e_single_thread	src/v/storage/tests/storage_e2e_single_thread		unit	https://buildkite.com/redpanda/redpanda/builds/79846#019c0a90-b168-4a99-8811-97a99b4ba3ef	FAIL	0/1

test results on build#79865

test_class	test_method	test_arguments	test_kind	job_url	test_status	passed	reason	test_history
coco_fixture	test_decommission		unit	https://buildkite.com/redpanda/redpanda/builds/79865#019c0b5c-d21b-494b-9e68-0b267ae121fa	FAIL	0/1

travisdowns · 2026-01-29T19:05:26Z

/ci-repeat 1
skip-redpanda-build
skip-units
skip-rebase
tests/rptest/tests/write_caching_fi_e2e_test.py::WriteCachingFailureInjectionE2ETest.test_crash_all@{"use_transactions":false}

StephanDollberg · 2026-01-29T19:04:17Z

src/v/bytes/tests/iobuf_tests.cc

+    buf2.append_fragments(iobuf::from("a"));
+    buf2.append_fragments(iobuf::from("b"));
+    buf2.append_fragments(iobuf::from("c"));
+    // buf2 = "abc"


remove or reword what it's trying to say? Should it be ==?

Well I guess it's like "this is the equivalent assignment" rather than an assertion, but regardless removed.

StephanDollberg · 2026-01-29T19:08:21Z

src/v/bytes/iobuf.cc

-          return !are_equal ? ss::stop_iteration::yes : ss::stop_iteration::no;
-      });
-    return are_equal;
+    return _size == o.size() && (*this <=> o) == std::strong_ordering::equal;


Other method calls size_bytes(). Shoudl this too?

Yes, better to use the method, fixed.

Add sv_eq_* and sv_cmp_* benchmark cases to measure performance of iobuf::operator== and operator<=> against string_view. Uses a templated cmp_bench function with rhs_type parameter to support both iobuf-vs-iobuf and iobuf-vs-string_view comparisons. Tests both single-fragment and fragmented iobufs at sizes 0, 1, 1024.

vbotbuildovich · 2026-01-30T20:30:49Z

/backport v25.3.x

travisdowns added 2 commits January 29, 2026 13:19

travisdowns requested review from StephanDollberg, Copilot and rockwotj January 29, 2026 16:21

github-actions bot added the area/redpanda label Jan 29, 2026

Copilot AI reviewed Jan 29, 2026

View reviewed changes

src/v/bytes/tests/iobuf_bench.cc Show resolved Hide resolved

rockwotj previously approved these changes Jan 29, 2026

View reviewed changes

StephanDollberg previously approved these changes Jan 29, 2026

View reviewed changes

travisdowns dismissed stale reviews from StephanDollberg and rockwotj via 75f1508 January 29, 2026 19:31

travisdowns force-pushed the td-iobuf-sv-cmp branch from 722dc19 to 75f1508 Compare January 29, 2026 19:31

travisdowns force-pushed the td-iobuf-sv-cmp branch from 75f1508 to 5752502 Compare January 29, 2026 20:05

StephanDollberg approved these changes Jan 29, 2026

View reviewed changes

travisdowns merged commit 00a4b08 into dev Jan 30, 2026
19 checks passed

travisdowns deleted the td-iobuf-sv-cmp branch January 30, 2026 20:30

vbotbuildovich mentioned this pull request Jan 30, 2026

[v25.3.x] iobuf: fix operator<=> and operator== for string_view comparisons #29477

Merged

Conversation

travisdowns commented Jan 29, 2026 • edited by rockwotj Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Benchmark results

Test plan

Backports Required

Release Notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

travisdowns commented Jan 29, 2026

Uh oh!

rockwotj commented Jan 29, 2026

Uh oh!

rockwotj left a comment

Choose a reason for hiding this comment

Uh oh!

travisdowns commented Jan 29, 2026

Uh oh!

vbotbuildovich commented Jan 29, 2026

Retry command for Build#79846

Uh oh!

vbotbuildovich commented Jan 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CI test results

Uh oh!

travisdowns commented Jan 29, 2026

Uh oh!

StephanDollberg Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

travisdowns Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

StephanDollberg Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

travisdowns Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vbotbuildovich commented Jan 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

travisdowns commented Jan 29, 2026 •

edited by rockwotj

Loading

vbotbuildovich commented Jan 29, 2026 •

edited

Loading