Skip to content

Commit 92d8ed8

Browse files
Eric Wonggitster
Eric Wong
authored andcommitted
oidtree: a crit-bit tree for odb_loose_cache
This saves 8K per `struct object_directory', meaning it saves around 800MB in my case involving 100K alternates (half or more of those alternates are unlikely to hold loose objects). This is implemented in two parts: a generic, allocation-free `cbtree' and the `oidtree' wrapper on top of it. The latter provides allocation using alloc_state as a memory pool to improve locality and reduce free(3) overhead. Unlike oid-array, the crit-bit tree does not require sorting. Performance is bound by the key length, for oidtree that is fixed at sizeof(struct object_id). There's no need to have 256 oidtrees to mitigate the O(n log n) overhead like we did with oid-array. Being a prefix trie, it is natively suited for expanding short object IDs via prefix-limited iteration in `find_short_object_filename'. On my busy workstation, p4205 performance seems to be roughly unchanged (+/-8%). Startup with 100K total alternates with no loose objects seems around 10-20% faster on a hot cache. (800MB in memory savings means more memory for the kernel FS cache). The generic cbtree implementation does impose some extra overhead for oidtree in that it uses memcmp(3) on "struct object_id" so it wastes cycles comparing 12 extra bytes on SHA-1 repositories. I've not yet explored reducing this overhead, but I expect there are many places in our code base where we'd want to investigate this. More information on crit-bit trees: https://cr.yp.to/critbit.html Signed-off-by: Eric Wong <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 90e07f0 commit 92d8ed8

12 files changed

+478
-30
lines changed

Makefile

+3
Original file line numberDiff line numberDiff line change
@@ -722,6 +722,7 @@ TEST_BUILTINS_OBJS += test-mergesort.o
722722
TEST_BUILTINS_OBJS += test-mktemp.o
723723
TEST_BUILTINS_OBJS += test-oid-array.o
724724
TEST_BUILTINS_OBJS += test-oidmap.o
725+
TEST_BUILTINS_OBJS += test-oidtree.o
725726
TEST_BUILTINS_OBJS += test-online-cpus.o
726727
TEST_BUILTINS_OBJS += test-parse-options.o
727728
TEST_BUILTINS_OBJS += test-parse-pathspec-file.o
@@ -845,6 +846,7 @@ LIB_OBJS += branch.o
845846
LIB_OBJS += bulk-checkin.o
846847
LIB_OBJS += bundle.o
847848
LIB_OBJS += cache-tree.o
849+
LIB_OBJS += cbtree.o
848850
LIB_OBJS += chdir-notify.o
849851
LIB_OBJS += checkout.o
850852
LIB_OBJS += chunk-format.o
@@ -940,6 +942,7 @@ LIB_OBJS += object.o
940942
LIB_OBJS += oid-array.o
941943
LIB_OBJS += oidmap.o
942944
LIB_OBJS += oidset.o
945+
LIB_OBJS += oidtree.o
943946
LIB_OBJS += pack-bitmap-write.o
944947
LIB_OBJS += pack-bitmap.o
945948
LIB_OBJS += pack-check.o

cbtree.c

+167
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
/*
2+
* crit-bit tree implementation, does no allocations internally
3+
* For more information on crit-bit trees: https://cr.yp.to/critbit.html
4+
* Based on Adam Langley's adaptation of Dan Bernstein's public domain code
5+
* git clone https://github.com/agl/critbit.git
6+
*/
7+
#include "cbtree.h"
8+
9+
static struct cb_node *cb_node_of(const void *p)
10+
{
11+
return (struct cb_node *)((uintptr_t)p - 1);
12+
}
13+
14+
/* locate the best match, does not do a final comparision */
15+
static struct cb_node *cb_internal_best_match(struct cb_node *p,
16+
const uint8_t *k, size_t klen)
17+
{
18+
while (1 & (uintptr_t)p) {
19+
struct cb_node *q = cb_node_of(p);
20+
uint8_t c = q->byte < klen ? k[q->byte] : 0;
21+
size_t direction = (1 + (q->otherbits | c)) >> 8;
22+
23+
p = q->child[direction];
24+
}
25+
return p;
26+
}
27+
28+
/* returns NULL if successful, existing cb_node if duplicate */
29+
struct cb_node *cb_insert(struct cb_tree *t, struct cb_node *node, size_t klen)
30+
{
31+
size_t newbyte, newotherbits;
32+
uint8_t c;
33+
int newdirection;
34+
struct cb_node **wherep, *p;
35+
36+
assert(!((uintptr_t)node & 1)); /* allocations must be aligned */
37+
38+
if (!t->root) { /* insert into empty tree */
39+
t->root = node;
40+
return NULL; /* success */
41+
}
42+
43+
/* see if a node already exists */
44+
p = cb_internal_best_match(t->root, node->k, klen);
45+
46+
/* find first differing byte */
47+
for (newbyte = 0; newbyte < klen; newbyte++) {
48+
if (p->k[newbyte] != node->k[newbyte])
49+
goto different_byte_found;
50+
}
51+
return p; /* element exists, let user deal with it */
52+
53+
different_byte_found:
54+
newotherbits = p->k[newbyte] ^ node->k[newbyte];
55+
newotherbits |= newotherbits >> 1;
56+
newotherbits |= newotherbits >> 2;
57+
newotherbits |= newotherbits >> 4;
58+
newotherbits = (newotherbits & ~(newotherbits >> 1)) ^ 255;
59+
c = p->k[newbyte];
60+
newdirection = (1 + (newotherbits | c)) >> 8;
61+
62+
node->byte = newbyte;
63+
node->otherbits = newotherbits;
64+
node->child[1 - newdirection] = node;
65+
66+
/* find a place to insert it */
67+
wherep = &t->root;
68+
for (;;) {
69+
struct cb_node *q;
70+
size_t direction;
71+
72+
p = *wherep;
73+
if (!(1 & (uintptr_t)p))
74+
break;
75+
q = cb_node_of(p);
76+
if (q->byte > newbyte)
77+
break;
78+
if (q->byte == newbyte && q->otherbits > newotherbits)
79+
break;
80+
c = q->byte < klen ? node->k[q->byte] : 0;
81+
direction = (1 + (q->otherbits | c)) >> 8;
82+
wherep = q->child + direction;
83+
}
84+
85+
node->child[newdirection] = *wherep;
86+
*wherep = (struct cb_node *)(1 + (uintptr_t)node);
87+
88+
return NULL; /* success */
89+
}
90+
91+
struct cb_node *cb_lookup(struct cb_tree *t, const uint8_t *k, size_t klen)
92+
{
93+
struct cb_node *p = cb_internal_best_match(t->root, k, klen);
94+
95+
return p && !memcmp(p->k, k, klen) ? p : NULL;
96+
}
97+
98+
struct cb_node *cb_unlink(struct cb_tree *t, const uint8_t *k, size_t klen)
99+
{
100+
struct cb_node **wherep = &t->root;
101+
struct cb_node **whereq = NULL;
102+
struct cb_node *q = NULL;
103+
size_t direction = 0;
104+
uint8_t c;
105+
struct cb_node *p = t->root;
106+
107+
if (!p) return NULL; /* empty tree, nothing to delete */
108+
109+
/* traverse to find best match, keeping link to parent */
110+
while (1 & (uintptr_t)p) {
111+
whereq = wherep;
112+
q = cb_node_of(p);
113+
c = q->byte < klen ? k[q->byte] : 0;
114+
direction = (1 + (q->otherbits | c)) >> 8;
115+
wherep = q->child + direction;
116+
p = *wherep;
117+
}
118+
119+
if (memcmp(p->k, k, klen))
120+
return NULL; /* no match, nothing unlinked */
121+
122+
/* found an exact match */
123+
if (whereq) /* update parent */
124+
*whereq = q->child[1 - direction];
125+
else
126+
t->root = NULL;
127+
return p;
128+
}
129+
130+
static enum cb_next cb_descend(struct cb_node *p, cb_iter fn, void *arg)
131+
{
132+
if (1 & (uintptr_t)p) {
133+
struct cb_node *q = cb_node_of(p);
134+
enum cb_next n = cb_descend(q->child[0], fn, arg);
135+
136+
return n == CB_BREAK ? n : cb_descend(q->child[1], fn, arg);
137+
} else {
138+
return fn(p, arg);
139+
}
140+
}
141+
142+
void cb_each(struct cb_tree *t, const uint8_t *kpfx, size_t klen,
143+
cb_iter fn, void *arg)
144+
{
145+
struct cb_node *p = t->root;
146+
struct cb_node *top = p;
147+
size_t i = 0;
148+
149+
if (!p) return; /* empty tree */
150+
151+
/* Walk tree, maintaining top pointer */
152+
while (1 & (uintptr_t)p) {
153+
struct cb_node *q = cb_node_of(p);
154+
uint8_t c = q->byte < klen ? kpfx[q->byte] : 0;
155+
size_t direction = (1 + (q->otherbits | c)) >> 8;
156+
157+
p = q->child[direction];
158+
if (q->byte < klen)
159+
top = p;
160+
}
161+
162+
for (i = 0; i < klen; i++) {
163+
if (p->k[i] != kpfx[i])
164+
return; /* "best" match failed */
165+
}
166+
cb_descend(top, fn, arg);
167+
}

cbtree.h

+56
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
/*
2+
* crit-bit tree implementation, does no allocations internally
3+
* For more information on crit-bit trees: https://cr.yp.to/critbit.html
4+
* Based on Adam Langley's adaptation of Dan Bernstein's public domain code
5+
* git clone https://github.com/agl/critbit.git
6+
*
7+
* This is adapted to store arbitrary data (not just NUL-terminated C strings
8+
* and allocates no memory internally. The user needs to allocate
9+
* "struct cb_node" and fill cb_node.k[] with arbitrary match data
10+
* for memcmp.
11+
* If "klen" is variable, then it should be embedded into "c_node.k[]"
12+
* Recursion is bound by the maximum value of "klen" used.
13+
*/
14+
#ifndef CBTREE_H
15+
#define CBTREE_H
16+
17+
#include "git-compat-util.h"
18+
19+
struct cb_node;
20+
struct cb_node {
21+
struct cb_node *child[2];
22+
/*
23+
* n.b. uint32_t for `byte' is excessive for OIDs,
24+
* we may consider shorter variants if nothing else gets stored.
25+
*/
26+
uint32_t byte;
27+
uint8_t otherbits;
28+
uint8_t k[FLEX_ARRAY]; /* arbitrary data */
29+
};
30+
31+
struct cb_tree {
32+
struct cb_node *root;
33+
};
34+
35+
enum cb_next {
36+
CB_CONTINUE = 0,
37+
CB_BREAK = 1
38+
};
39+
40+
#define CBTREE_INIT { .root = NULL }
41+
42+
static inline void cb_init(struct cb_tree *t)
43+
{
44+
t->root = NULL;
45+
}
46+
47+
struct cb_node *cb_lookup(struct cb_tree *, const uint8_t *k, size_t klen);
48+
struct cb_node *cb_insert(struct cb_tree *, struct cb_node *, size_t klen);
49+
struct cb_node *cb_unlink(struct cb_tree *t, const uint8_t *k, size_t klen);
50+
51+
typedef enum cb_next (*cb_iter)(struct cb_node *, void *arg);
52+
53+
void cb_each(struct cb_tree *, const uint8_t *kpfx, size_t klen,
54+
cb_iter, void *arg);
55+
56+
#endif /* CBTREE_H */

object-file.c

+12-11
Original file line numberDiff line numberDiff line change
@@ -1173,7 +1173,7 @@ static int quick_has_loose(struct repository *r,
11731173

11741174
prepare_alt_odb(r);
11751175
for (odb = r->objects->odb; odb; odb = odb->next) {
1176-
if (oid_array_lookup(odb_loose_cache(odb, oid), oid) >= 0)
1176+
if (oidtree_contains(odb_loose_cache(odb, oid), oid))
11771177
return 1;
11781178
}
11791179
return 0;
@@ -2452,11 +2452,11 @@ int for_each_loose_object(each_loose_object_fn cb, void *data,
24522452
static int append_loose_object(const struct object_id *oid, const char *path,
24532453
void *data)
24542454
{
2455-
oid_array_append(data, oid);
2455+
oidtree_insert(data, oid);
24562456
return 0;
24572457
}
24582458

2459-
struct oid_array *odb_loose_cache(struct object_directory *odb,
2459+
struct oidtree *odb_loose_cache(struct object_directory *odb,
24602460
const struct object_id *oid)
24612461
{
24622462
int subdir_nr = oid->hash[0];
@@ -2472,24 +2472,25 @@ struct oid_array *odb_loose_cache(struct object_directory *odb,
24722472

24732473
bitmap = &odb->loose_objects_subdir_seen[word_index];
24742474
if (*bitmap & mask)
2475-
return &odb->loose_objects_cache[subdir_nr];
2476-
2475+
return odb->loose_objects_cache;
2476+
if (!odb->loose_objects_cache) {
2477+
ALLOC_ARRAY(odb->loose_objects_cache, 1);
2478+
oidtree_init(odb->loose_objects_cache);
2479+
}
24772480
strbuf_addstr(&buf, odb->path);
24782481
for_each_file_in_obj_subdir(subdir_nr, &buf,
24792482
append_loose_object,
24802483
NULL, NULL,
2481-
&odb->loose_objects_cache[subdir_nr]);
2484+
odb->loose_objects_cache);
24822485
*bitmap |= mask;
24832486
strbuf_release(&buf);
2484-
return &odb->loose_objects_cache[subdir_nr];
2487+
return odb->loose_objects_cache;
24852488
}
24862489

24872490
void odb_clear_loose_cache(struct object_directory *odb)
24882491
{
2489-
int i;
2490-
2491-
for (i = 0; i < ARRAY_SIZE(odb->loose_objects_cache); i++)
2492-
oid_array_clear(&odb->loose_objects_cache[i]);
2492+
oidtree_clear(odb->loose_objects_cache);
2493+
FREE_AND_NULL(odb->loose_objects_cache);
24932494
memset(&odb->loose_objects_subdir_seen, 0,
24942495
sizeof(odb->loose_objects_subdir_seen));
24952496
}

object-name.c

+11-17
Original file line numberDiff line numberDiff line change
@@ -87,27 +87,21 @@ static void update_candidates(struct disambiguate_state *ds, const struct object
8787

8888
static int match_hash(unsigned, const unsigned char *, const unsigned char *);
8989

90+
static enum cb_next match_prefix(const struct object_id *oid, void *arg)
91+
{
92+
struct disambiguate_state *ds = arg;
93+
/* no need to call match_hash, oidtree_each did prefix match */
94+
update_candidates(ds, oid);
95+
return ds->ambiguous ? CB_BREAK : CB_CONTINUE;
96+
}
97+
9098
static void find_short_object_filename(struct disambiguate_state *ds)
9199
{
92100
struct object_directory *odb;
93101

94-
for (odb = ds->repo->objects->odb; odb && !ds->ambiguous; odb = odb->next) {
95-
int pos;
96-
struct oid_array *loose_objects;
97-
98-
loose_objects = odb_loose_cache(odb, &ds->bin_pfx);
99-
pos = oid_array_lookup(loose_objects, &ds->bin_pfx);
100-
if (pos < 0)
101-
pos = -1 - pos;
102-
while (!ds->ambiguous && pos < loose_objects->nr) {
103-
const struct object_id *oid;
104-
oid = loose_objects->oid + pos;
105-
if (!match_hash(ds->len, ds->bin_pfx.hash, oid->hash))
106-
break;
107-
update_candidates(ds, oid);
108-
pos++;
109-
}
110-
}
102+
for (odb = ds->repo->objects->odb; odb && !ds->ambiguous; odb = odb->next)
103+
oidtree_each(odb_loose_cache(odb, &ds->bin_pfx),
104+
&ds->bin_pfx, ds->len, match_prefix, ds);
111105
}
112106

113107
static int match_hash(unsigned len, const unsigned char *a, const unsigned char *b)

object-store.h

+3-2
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include "thread-utils.h"
1010
#include "khash.h"
1111
#include "dir.h"
12+
#include "oidtree.h"
1213

1314
struct object_directory {
1415
struct object_directory *next;
@@ -23,7 +24,7 @@ struct object_directory {
2324
* Be sure to call odb_load_loose_cache() before using.
2425
*/
2526
uint32_t loose_objects_subdir_seen[8]; /* 256 bits */
26-
struct oid_array loose_objects_cache[256];
27+
struct oidtree *loose_objects_cache;
2728

2829
/*
2930
* Path to the alternative object store. If this is a relative path,
@@ -59,7 +60,7 @@ void add_to_alternates_memory(const char *dir);
5960
* Populate and return the loose object cache array corresponding to the
6061
* given object ID.
6162
*/
62-
struct oid_array *odb_loose_cache(struct object_directory *odb,
63+
struct oidtree *odb_loose_cache(struct object_directory *odb,
6364
const struct object_id *oid);
6465

6566
/* Empty the loose object cache for the specified object directory. */

0 commit comments

Comments
 (0)