diff --git a/src/BTrees/BTreeModuleTemplate.c b/src/BTrees/BTreeModuleTemplate.c
index 5683cb9fc58e61b021365c763c5722cb752b35d2..909400250dbaf168109f36d504c44b0768f54413 100755
--- a/src/BTrees/BTreeModuleTemplate.c
+++ b/src/BTrees/BTreeModuleTemplate.c
@@ -147,6 +147,32 @@ staticforward PyExtensionClass BTreeType;
#define BTREE(O) ((BTree*)(O))
+/* Use BTREE_SEARCH to find which child pointer to follow.
+ * RESULT An int lvalue to hold the index i such that SELF->data[i].child
+ * is the correct node to search next.
+ * SELF A pointer to a BTree node.
+ * KEY The key you're looking for, of type KEY_TYPE.
+ * ONERROR What to do if key comparison raises an exception; for example,
+ * perhaps 'return NULL'.
+ *
+ * See Maintainer.txt for discussion: this is optimized in subtle ways.
+ * It's recommended that you call this at the start of a routine, waiting
+ * to check for self->len == 0 after.
+ */
+#define BTREE_SEARCH(RESULT, SELF, KEY, ONERROR) { \
+ int _lo = 0; \
+ int _hi = (SELF)->len; \
+ int _i, _cmp; \
+ for (_i = _hi >> 1; _i > _lo; _i = (_lo + _hi) >> 1) { \
+ TEST_KEY_SET_OR(_cmp, (SELF)->data[_i].key, (KEY)) \
+ ONERROR; \
+ if (_cmp < 0) _lo = _i; \
+ else if (_cmp > 0) _hi = _i; \
+ else /* equal */ break; \
+ } \
+ (RESULT) = _i; \
+}
+
typedef struct SetIteration_s
{
PyObject *set;
@@ -323,7 +349,7 @@ static char BTree_module_documentation[] =
"\n"
MASTER_ID
BTREEITEMSTEMPLATE_C
-"$Id: BTreeModuleTemplate.c,v 1.26 2002/05/31 17:56:59 tim_one Exp $\n"
+"$Id: BTreeModuleTemplate.c,v 1.27 2002/05/31 20:01:16 tim_one Exp $\n"
BTREETEMPLATE_C
BUCKETTEMPLATE_C
KEYMACROS_H
diff --git a/src/BTrees/BTreeTemplate.c b/src/BTrees/BTreeTemplate.c
index a2118b821216661d2c0c080f550cc7b69b9b0d63..8eaec5517c1f147a6a702956b7384a04bf733415 100755
--- a/src/BTrees/BTreeTemplate.c
+++ b/src/BTrees/BTreeTemplate.c
@@ -12,7 +12,7 @@
****************************************************************************/
-#define BTREETEMPLATE_C "$Id: BTreeTemplate.c,v 1.32 2002/05/31 17:56:59 tim_one Exp $\n"
+#define BTREETEMPLATE_C "$Id: BTreeTemplate.c,v 1.33 2002/05/31 20:01:16 tim_one Exp $\n"
/*
** _BTree_get
@@ -21,8 +21,8 @@
static PyObject *
_BTree_get(BTree *self, PyObject *keyarg, int has_key)
{
- int min, max, i, cmp, copied=1;
- PyObject *r;
+ int min, copied=1;
+ PyObject *r = NULL;
KEY_TYPE key;
COPY_KEY_FROM_ARG(key, keyarg, copied);
@@ -30,38 +30,25 @@ _BTree_get(BTree *self, PyObject *keyarg, int has_key)
PER_USE_OR_RETURN(self, NULL);
+ BTREE_SEARCH(min, self, key, goto Error);
if (self->len)
{
- for (min=0, max=self->len, i=max/2; max-min > 1; i=(min+max)/2)
- {
- TEST_KEY_SET_OR(cmp, self->data[i].key, key) return NULL;
- if (cmp < 0) min=i;
- else if (cmp == 0)
- {
- min=i;
- break;
- }
- else max=i;
- }
-
if (SameType_Check(self, self->data[min].child))
- r=_BTree_get( BTREE(self->data[min].child), keyarg,
- has_key ? has_key + 1: 0);
+ r = _BTree_get( BTREE(self->data[min].child), keyarg,
+ has_key ? has_key + 1: 0);
else
- r=_bucket_get(BUCKET(self->data[min].child), keyarg,
- has_key ? has_key + 1: 0);
+ r = _bucket_get(BUCKET(self->data[min].child), keyarg,
+ has_key ? has_key + 1: 0);
}
else
{ /* No data */
UNLESS (has_key)
- {
- PyErr_SetObject(PyExc_KeyError, keyarg);
- r=NULL;
- }
+ PyErr_SetObject(PyExc_KeyError, keyarg);
else
- r=PyInt_FromLong(0);
+ r = PyInt_FromLong(0);
}
+Error:
PER_ALLOW_DEACTIVATION(self);
PER_ACCESSED(self);
return r;
diff --git a/src/BTrees/Maintainer.txt b/src/BTrees/Maintainer.txt
index ffe4ec2c2d32838b5488247da1c39e980d37cf03..34bfc612d9b54a66bbe7f5ac2cd43cd885bd724d 100644
--- a/src/BTrees/Maintainer.txt
+++ b/src/BTrees/Maintainer.txt
@@ -171,3 +171,92 @@ More or less random bits of helpful info.
mostly to "even out" pickle sizes in storage. That's why, e.g.,
an IIBTree has larger values than an OOBTree: pickles store ints
more efficiently than they can store arbitrary Python objects.
+
+
+The BTREE_SEARCH Macro
+======================
+For notational ease, consider a fixed BTree node x, and let
+
+ K(i) mean x->data.key[i]
+ C(i) mean all the keys reachable from x->data.child[i]
+
+For each i in 0 to x->len-1 inclusive,
+
+ K(i) <= C(i) < K(i+1)
+
+is a BTree node invariant, where we pretend that K(0) holds a key
+smaller than any possible key, and K(x->len) holds a key larger
+than any possible key. (Note that K(x->len) doesn't actually exist,
+and K(0) is never used although space for it exists in non-empty
+BTree nodes.)
+
+When searching for a key k, then, the child pointer we want to follow
+is the one at index i such that K(i) <= k < K(i+1). There can be
+only one such i, since the keys are strictly increasing. And there is
+at *least* one such i provided the tree isn't empty. For the moment,
+assume the tree isn't empty (we'll get back to that later).
+
+The macro's chief loop invariant is
+
+ K(lo) < k < K(hi)
+
+This holds trivially at the start, since lo is set to 0 ahd hi to
+x->len, and we pretend K(0) is minus infinity and K(len) is plus
+infinity. Inside the loop, if K(i) < k we set lo to i, and if
+K(i) > k we set hi to i. These obviously preserve the invariant.
+If K(i) == k, the loop breaks and sets the result to i, and since
+K(i) == k in that case i is obviously the correct result.
+
+What if the key isn't present? lo and hi move toward each other,
+narrowing the range, until eventually lo+1 == hi. At that point,
+i = (lo+hi)/2 = (lo+lo+1)/2 = lo + 1/2 = lo, so that:
+
+1. The loop's "i > lo" test is false, so the loop ends then.
+
+and
+
+2. The invariant still holds, so K(i) < k < K(i+1), and i is again
+ the correct answer.
+
+Can we get out of the loop too early? No: if hi = lo + d for some d
+greater than 1, then i = (lo+lo+d)/2 = lo + d/2, and d/2 is at least 1
+since d is at least 2: i is strictly greater than lo then, and the
+loop continues.
+
+Can lo==hi? Yes, but only if the node is empty. Then i, lo and hi
+all start out as 0, and the loop exits immediately. If the loop
+isn't empty, then lo and hi start out with different values. Whenever
+lo and hi have different values, lo <= (lo + hi)/2 < hi, so i and lo
+are strictly smaller than hi, so setting either lo or hi to i leaves
+the new lo strictly smaller than the new hi.
+
+Can the loop fail to terminate? No: by the above, when lo < hi-1,
+lo < i=(lo+hi)/2 < hi, so setting either lo or hi to i leaves the
+new lo and hi strictly closer to each other than were the old lo and
+hi.
+
+Optimization points:
+
++ Division by 2 is done via shift rather via "/2". These are
+ signed ints, and almost all C compilers treat signed int division
+ as truncating, and shifting is not the same as truncation for
+ signed int division. The compiler has no way to know these values
+ aren't negative, so has to generate longer-winded code for "/2".
+ But we know these values aren't negative, and exploit it.
+
++ The order of _cmp comparisons matters. We're in an interior
+ BTree node, and are looking at only a tiny fraction of all the
+ keys that exist. So finding the key exactly in this node is
+ unlikely, and checking _cmp == 0 is a waste of time to the same
+ extent. It doesn't matter whether we check for _cmp < 0 or
+ _cmp > 0 first, so long as we do both before worrying about
+ equality.
+
++ At the start of a routine, it's better to run this macro even
+ if x->len is 0 (check for that afterwards). We just called a
+ function and so probably drained the pipeline. If the first thing
+ we do then is read up self->len and check it against 0, we just
+ sit there waiting for the data to get read up, and then another
+ immediate test-and-branch, and for a very unlikely case (BTree
+ nodes are rarely empty). It's better to get into the loop right
+ away so the normal case makes progress ASAP.