From: Casey Bodley <cbodley@redhat.com>
Date: Mon, 30 Mar 2020 16:46:33 +0000 (-0400)
Subject: bufferlist: use iterators for comparison operators
X-Git-Tag: v16.1.0~2495^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=6db17baf914f25dc61b1f40f54a50b6290989b75;p=ceph.git

bufferlist: use iterators for comparison operators

bufferlist's operator[] is not constant time! use iterators to avoid
these costly indexing operations

Signed-off-by: Casey Bodley <cbodley@redhat.com>
---

diff --git a/src/include/buffer.h b/src/include/buffer.h
index 9e0c2fd4d28..c8805cc5120 100644
--- a/src/include/buffer.h
+++ b/src/include/buffer.h
@@ -1230,44 +1230,40 @@ inline namespace v15_2_0 {
     }
   };
 
-inline bool operator>(const bufferlist& l, const bufferlist& r) {
-  for (unsigned p = 0; ; p++) {
-    if (l.length() > p && r.length() == p) return true;
-    if (l.length() == p) return false;
-    if (l[p] > r[p]) return true;
-    if (l[p] < r[p]) return false;
-  }
+inline bool operator==(const bufferlist &lhs, const bufferlist &rhs) {
+  if (lhs.length() != rhs.length())
+    return false;
+  return std::equal(lhs.begin(), lhs.end(), rhs.begin());
 }
-inline bool operator>=(const bufferlist& l, const bufferlist& r) {
-  for (unsigned p = 0; ; p++) {
-    if (l.length() > p && r.length() == p) return true;
-    if (r.length() == p && l.length() == p) return true;
-    if (l.length() == p && r.length() > p) return false;
-    if (l[p] > r[p]) return true;
-    if (l[p] < r[p]) return false;
+
+inline bool operator<(const bufferlist& lhs, const bufferlist& rhs) {
+  auto l = lhs.begin(), r = rhs.begin();
+  for (; l != lhs.end() && r != rhs.end(); ++l, ++r) {
+    if (*l < *r) return true;
+    if (*l > *r) return false;
   }
+  return (l == lhs.end()) && (r != rhs.end()); // lhs.length() < rhs.length()
 }
 
-inline bool operator==(const bufferlist &l, const bufferlist &r) {
-  if (l.length() != r.length())
-    return false;
-  for (unsigned p = 0; p < l.length(); p++) {
-    if (l[p] != r[p])
-      return false;
+inline bool operator<=(const bufferlist& lhs, const bufferlist& rhs) {
+  auto l = lhs.begin(), r = rhs.begin();
+  for (; l != lhs.end() && r != rhs.end(); ++l, ++r) {
+    if (*l < *r) return true;
+    if (*l > *r) return false;
   }
-  return true;
+  return l == lhs.end(); // lhs.length() <= rhs.length()
 }
+
 inline bool operator!=(const bufferlist &l, const bufferlist &r) {
   return !(l == r);
 }
-inline bool operator<(const bufferlist& l, const bufferlist& r) {
-  return r > l;
+inline bool operator>(const bufferlist& lhs, const bufferlist& rhs) {
+  return rhs < lhs;
 }
-inline bool operator<=(const bufferlist& l, const bufferlist& r) {
-  return r >= l;
+inline bool operator>=(const bufferlist& lhs, const bufferlist& rhs) {
+  return rhs <= lhs;
 }
 
-
 std::ostream& operator<<(std::ostream& out, const buffer::ptr& bp);
 
 std::ostream& operator<<(std::ostream& out, const buffer::raw &r);
diff --git a/src/test/bufferlist.cc b/src/test/bufferlist.cc
index 37de45f0e31..eb40b6483d9 100644
--- a/src/test/bufferlist.cc
+++ b/src/test/bufferlist.cc
@@ -2540,8 +2540,9 @@ TEST(BufferList, crc32c_append_perf) {
 TEST(BufferList, compare) {
   bufferlist a;
   a.append("A");
-  bufferlist ab;
-  ab.append("AB");
+  bufferlist ab; // AB in segments
+  ab.append(bufferptr("A", 1));
+  ab.append(bufferptr("B", 1));
   bufferlist ac;
   ac.append("AC");
   //