From 99a805517e770809a4e0b638b6aa4628a3d15efa Mon Sep 17 00:00:00 2001
From: Russell Sears <russell_sears@apple.com>
Date: Mon, 30 Mar 2020 16:13:37 -0700
Subject: [PATCH] reduce the number of calls to operator< made by lower_bound
 and upper_bound #2877

---
 fdbclient/VersionedMap.h | 45 ++++++++++++++++++++++++++++++----------
 flow/IndexedSet.h        | 14 +++++++++----
 2 files changed, 44 insertions(+), 15 deletions(-)

diff --git a/fdbclient/VersionedMap.h b/fdbclient/VersionedMap.h
index 19fc1b122e..d18943855c 100644
--- a/fdbclient/VersionedMap.h
+++ b/fdbclient/VersionedMap.h
@@ -114,30 +114,53 @@ namespace PTreeImpl {
 		return contains(p->child(!less, at), at, x);
 	}
 
+	// TODO: Remove the number of invocations of operator<, and replace with something closer to memcmp.
+	// and same for upper_bound.
 	template<class T, class X>
-	void lower_bound(const Reference<PTree<T>>& p, Version at, const X& x, std::vector<const PTree<T>*>& f){
+	void lower_bound(const Reference<PTree<T>>& p, Version at, const X& x, std::vector<const PTree<T>*>& f, std::vector<bool>& lessThan){
 		if (!p) {
-			while (f.size() && !(x < f.back()->data))
+			while (f.size() && !(lessThan.back())) {
 				f.pop_back();
+				lessThan.pop_back();
+			}
 			return;
 		}
 		f.push_back(p.getPtr());
 		bool less = x < p->data;
+		lessThan.push_back(less);
 		if (!less && !(p->data<x)) return;  // x == p->data
-		lower_bound(p->child(!less, at), at, x, f);
+		lower_bound(p->child(!less, at), at, x, f, lessThan);
+	}
+
+	template<class T, class X>
+	void lower_bound(const Reference<PTree<T>>& p, Version at, const X& x, std::vector<const PTree<T>*>& f) {
+		assert(!f.size());
+		std::vector<bool> lessThan;
+		lower_bound(p, at, x, f, lessThan);
+	}
+
+	template<class T, class X>
+	void upper_bound(const Reference<PTree<T>>& p, Version at, const X& x, std::vector<const PTree<T>*>& f, std::vector<bool>& lessThan){
+		if (!p) {
+			while (f.size() && !(lessThan.back())) {
+				f.pop_back();
+				lessThan.pop_back();
+			}
+			return;
+		}
+		f.push_back(p.getPtr());
+		bool less = x < p->data;
+		lessThan.push_back(less);
+		upper_bound(p->child(!less, at), at, x, f, lessThan);
 	}
 
 	template<class T, class X>
 	void upper_bound(const Reference<PTree<T>>& p, Version at, const X& x, std::vector<const PTree<T>*>& f){
-		if (!p) {
-			while (f.size() && !(x < f.back()->data))
-				f.pop_back();
-			return;
-		}
-		f.push_back(p.getPtr());
-		upper_bound(p->child(!(x < p->data), at), at, x, f);
+		assert(!f.size());
+		std::vector<bool> lessThan;
+		upper_bound(p, at, x, f, lessThan);
 	}
-	
+
 	template<class T, bool forward>
 	void move(Version at, std::vector<const PTree<T>*>& f){
 		ASSERT(f.size());
diff --git a/flow/IndexedSet.h b/flow/IndexedSet.h
index 4ed75056e5..99188c13ae 100644
--- a/flow/IndexedSet.h
+++ b/flow/IndexedSet.h
@@ -700,6 +700,7 @@ int IndexedSet<T,Metric>::insert(const std::vector<std::pair<T,Metric>>& dataVec
 			// traverse to find insert point
 			bool foundNode = false;
 			while (true){
+				// TODO: remove implicit double invocation of memcmp
 				d = t->data < data;
 				if (!d)
 					blockEnd = t;
@@ -995,6 +996,7 @@ template <class Key>
 typename IndexedSet<T,Metric>::iterator IndexedSet<T,Metric>::find(const Key &key) const {
 	Node* t = root;
 	while (t){
+		// TODO: avoid indirect double invocation of memcmp
 		int d = t->data < key;
 		if (!d && !(key < t->data)) // t->data == key
 			return iterator(t);
@@ -1009,13 +1011,15 @@ template <class Key>
 typename IndexedSet<T,Metric>::iterator IndexedSet<T,Metric>::lower_bound(const Key &key) const {
 	Node* t = root;
 	if (!t) return iterator();
+	bool less;
 	while (true) {
-		Node *n = t->child[ t->data < key ];
+		less = t->data < key;
+		Node *n = t->child[less];
 		if (!n) break;
 		t = n;
 	}
 
-	if (t->data < key)
+	if (less)
 		moveIterator<1>(t);
 
 	return iterator(t);
@@ -1027,13 +1031,15 @@ template <class Key>
 typename IndexedSet<T,Metric>::iterator IndexedSet<T,Metric>::upper_bound(const Key &key) const {
 	Node* t = root;
 	if (!t) return iterator();
+	bool not_less;
 	while (true) {
-		Node *n = t->child[ !(key < t->data) ];
+		not_less = !(key < t->data);
+		Node *n = t->child[not_less];
 		if (!n) break;
 		t = n;
 	}
 
-	if (!(key < t->data))
+	if (not_less)
 		moveIterator<1>(t);
 
 	return iterator(t);