Skip to content

Commit 192ecff

Browse files
rockwotjrockwotj
authored
Merge pull request #29477 from vbotbuildovich/backport-pr-29461-v25.3.x-402
2 parents f1dbaaa + de679bc commit 192ecff

3 files changed

Lines changed: 129 additions & 42 deletions

File tree

‎src/v/bytes/iobuf.cc‎

Lines changed: 17 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -182,43 +182,27 @@ std::strong_ordering iobuf::operator<=>(const iobuf& o) const {
182182
}
183183

184184
bool iobuf::operator==(std::string_view o) const {
185-
if (_size != o.size()) {
186-
return false;
187-
}
188-
bool are_equal = true;
189-
std::string_view::size_type n = 0;
190-
auto in = iobuf::iterator_consumer(cbegin(), cend());
191-
std::ignore = in.consume(
192-
size_bytes(), [&are_equal, &o, &n](const char* src, size_t fg_sz) {
193-
/// Both strings are equiv in total size, so its safe to assume
194-
/// the next chunk to compare is the remaining to cmp or the
195-
/// fragment size
196-
const auto size = std::min((o.size() - n), fg_sz);
197-
std::string_view a_view(src, size);
198-
std::string_view b_view(o.data() + n, size);
199-
n += size;
200-
are_equal &= (a_view == b_view);
201-
return !are_equal ? ss::stop_iteration::yes : ss::stop_iteration::no;
202-
});
203-
return are_equal;
185+
return size_bytes() == o.size()
186+
&& (*this <=> o) == std::strong_ordering::equal;
204187
}
205188

206189
std::strong_ordering iobuf::operator<=>(std::string_view o) const {
207-
std::strong_ordering cmp = std::strong_ordering::equal;
208-
auto in = iobuf::iterator_consumer(cbegin(), cend());
209-
std::string_view other = o;
210-
std::ignore = in.consume(
211-
std::min(size_bytes(), o.size()),
212-
[&cmp, &other](const char* src, size_t fg_sz) {
213-
cmp = std::string_view(src, fg_sz) <=> other;
214-
other.remove_prefix(std::min(fg_sz, other.size()));
215-
return cmp == std::strong_ordering::equal ? ss::stop_iteration::yes
216-
: ss::stop_iteration::no;
217-
});
218-
if (cmp == std::strong_ordering::equal) {
219-
cmp = size_bytes() <=> o.size();
190+
auto other = o;
191+
// first compare the common length prefix
192+
for (const auto& frag : *this) {
193+
auto compare_len = std::min(frag.size(), other.size());
194+
std::strong_ordering cmp = std::string_view(frag.get(), compare_len)
195+
<=> other.substr(0, compare_len);
196+
if (cmp != std::strong_ordering::equal) {
197+
return cmp;
198+
}
199+
other.remove_prefix(compare_len);
200+
if (other.empty()) {
201+
break;
202+
}
220203
}
221-
return cmp;
204+
// prefix was equal, only size matters now
205+
return size_bytes() <=> o.size();
222206
}
223207

224208
/**

‎src/v/bytes/tests/iobuf_bench.cc‎

Lines changed: 47 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -49,21 +49,42 @@ size_t move_bench() {
4949
return inner_iters * 2;
5050
}
5151

52-
template<size_t Size, typename cmp_fn, bool same>
52+
// Overloaded make_rhs: convert iobuf to the desired RHS type
53+
template<typename T>
54+
T make_rhs(iobuf&& src);
55+
56+
template<>
57+
iobuf make_rhs<iobuf>(iobuf&& src) {
58+
return std::move(src);
59+
}
60+
61+
template<>
62+
ss::sstring make_rhs<ss::sstring>(iobuf&& src) {
63+
return src.linearize_to_string();
64+
}
65+
66+
// Get the view type for comparison (iobuf& or string_view)
67+
const iobuf& as_cmp_arg(const iobuf& b) { return b; }
68+
std::string_view as_cmp_arg(const ss::sstring& b) { return b; }
69+
70+
template<size_t Size, typename cmp_fn, bool same, typename rhs_type = iobuf>
5371
size_t cmp_bench() {
54-
iobuf a = make_iobuf(Size);
55-
iobuf a_copy = a.copy();
56-
iobuf b = make_iobuf(Size, !same);
57-
iobuf b_copy;
58-
for (const auto& frag : b) {
72+
// LHS: iobuf (single fragment and fragmented variants)
73+
iobuf lhs = make_iobuf(Size);
74+
iobuf lhs_fragmented;
75+
for (const auto& frag : lhs) {
5976
for (char c : std::string_view(frag.get(), frag.size())) {
60-
b_copy.append(&c, 1);
77+
lhs_fragmented.append(&c, 1);
6178
}
6279
}
80+
81+
// RHS: converted to target type (iobuf or ss::sstring)
82+
auto rhs = make_rhs<rhs_type>(make_iobuf(Size, !same));
83+
6384
perf_tests::start_measuring_time();
6485
for (auto i = inner_iters; i--;) {
65-
perf_tests::do_not_optimize(cmp_fn{}(a, b));
66-
perf_tests::do_not_optimize(cmp_fn{}(a_copy, b_copy));
86+
perf_tests::do_not_optimize(cmp_fn{}(lhs, as_cmp_arg(rhs)));
87+
perf_tests::do_not_optimize(cmp_fn{}(lhs_fragmented, as_cmp_arg(rhs)));
6788
}
6889
perf_tests::stop_measuring_time();
6990
return inner_iters * 2;
@@ -147,3 +168,20 @@ PERF_TEST(iobuf, eq_bench_large_same) {
147168
PERF_TEST(iobuf, append_bench_small) { return append_bench<1'000, 4>(); }
148169
PERF_TEST(iobuf, append_bench_medium) { return append_bench<1'000, 40_KiB>(); }
149170
PERF_TEST(iobuf, append_bench_large) { return append_bench<1'000, 400_KiB>(); }
171+
172+
// iobuf vs string_view comparisons
173+
// clang-format off
174+
PERF_TEST(iobuf, sv_eq_0000) { return cmp_bench< 0, std::equal_to<>, false, ss::sstring>(); }
175+
PERF_TEST(iobuf, sv_eq_0001) { return cmp_bench< 1, std::equal_to<>, false, ss::sstring>(); }
176+
PERF_TEST(iobuf, sv_eq_1024) { return cmp_bench<1024, std::equal_to<>, false, ss::sstring>(); }
177+
PERF_TEST(iobuf, sv_eq_0000_same) { return cmp_bench< 0, std::equal_to<>, true, ss::sstring>(); }
178+
PERF_TEST(iobuf, sv_eq_0001_same) { return cmp_bench< 1, std::equal_to<>, true, ss::sstring>(); }
179+
PERF_TEST(iobuf, sv_eq_1024_same) { return cmp_bench<1024, std::equal_to<>, true, ss::sstring>(); }
180+
181+
PERF_TEST(iobuf, sv_cmp_0000) { return cmp_bench< 0, std::less<>, false, ss::sstring>(); }
182+
PERF_TEST(iobuf, sv_cmp_0001) { return cmp_bench< 1, std::less<>, false, ss::sstring>(); }
183+
PERF_TEST(iobuf, sv_cmp_1024) { return cmp_bench<1024, std::less<>, false, ss::sstring>(); }
184+
PERF_TEST(iobuf, sv_cmp_0000_same) { return cmp_bench< 0, std::less<>, true, ss::sstring>(); }
185+
PERF_TEST(iobuf, sv_cmp_0001_same) { return cmp_bench< 1, std::less<>, true, ss::sstring>(); }
186+
PERF_TEST(iobuf, sv_cmp_1024_same) { return cmp_bench<1024, std::less<>, true, ss::sstring>(); }
187+
// clang-format on

‎src/v/bytes/tests/iobuf_tests.cc‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -856,3 +856,68 @@ SEASTAR_THREAD_TEST_CASE(iobuf_linearize) {
856856
iobuf large = iobuf::from(std::string(128_KiB + 1, 'a'));
857857
BOOST_CHECK_THROW(large.linearize_to_string(), std::runtime_error);
858858
}
859+
860+
SEASTAR_THREAD_TEST_CASE(iobuf_spaceship_string_view) {
861+
// Multi-fragment iobuf compared against string_view, these are potential
862+
// sources of bugs as the comparison is done fragment by fragment.
863+
iobuf buf = iobuf::from("ab");
864+
buf.append_fragments(iobuf::from("cd"));
865+
866+
// Equal comparison
867+
BOOST_CHECK(
868+
(buf <=> std::string_view("abcd")) == std::strong_ordering::equal);
869+
870+
// Less than - iobuf is less
871+
BOOST_CHECK(
872+
(buf <=> std::string_view("abce")) == std::strong_ordering::less);
873+
874+
// Greater than - iobuf is greater
875+
BOOST_CHECK(
876+
(buf <=> std::string_view("abcc")) == std::strong_ordering::greater);
877+
878+
// Size difference - iobuf shorter
879+
BOOST_CHECK(
880+
(buf <=> std::string_view("abcde")) == std::strong_ordering::less);
881+
882+
// Size difference - iobuf longer
883+
BOOST_CHECK(
884+
(buf <=> std::string_view("abc")) == std::strong_ordering::greater);
885+
886+
// Test with many small fragments - difference in second fragment
887+
iobuf buf2;
888+
buf2.append_fragments(iobuf::from("a"));
889+
buf2.append_fragments(iobuf::from("b"));
890+
buf2.append_fragments(iobuf::from("c"));
891+
892+
BOOST_CHECK(
893+
(buf2 <=> std::string_view("abc")) == std::strong_ordering::equal);
894+
BOOST_CHECK(
895+
(buf2 <=> std::string_view("abd")) == std::strong_ordering::less);
896+
BOOST_CHECK(
897+
(buf2 <=> std::string_view("abb")) == std::strong_ordering::greater);
898+
899+
// Test mismatch in middle of second fragment
900+
iobuf buf3;
901+
buf3.append_fragments(iobuf::from("xx"));
902+
buf3.append_fragments(iobuf::from("yy"));
903+
904+
BOOST_CHECK(
905+
(buf3 <=> std::string_view("xxyy")) == std::strong_ordering::equal);
906+
BOOST_CHECK(
907+
(buf3 <=> std::string_view("xxyz")) == std::strong_ordering::less);
908+
BOOST_CHECK(
909+
(buf3 <=> std::string_view("xxyx")) == std::strong_ordering::greater);
910+
911+
// Past bug reproducers
912+
iobuf buf4;
913+
buf4.append_fragments(iobuf::from("b"));
914+
buf4.append_fragments(iobuf::from("a"));
915+
BOOST_CHECK(
916+
(buf4 <=> std::string_view("ax")) == std::strong_ordering::greater);
917+
918+
iobuf buf5;
919+
buf5.append_fragments(iobuf::from("a"));
920+
buf5.append_fragments(iobuf::from("z"));
921+
BOOST_CHECK(
922+
(buf5 <=> std::string_view("bx")) == std::strong_ordering::less);
923+
}

0 commit comments

Comments
 (0)