Hi René, On Fri, Apr 6, 2018 at 9:58 PM, René Scharfe <l.s.r@xxxxxx> wrote: > Am 07.04.2018 um 01:21 schrieb Stefan Beller: >> This applies on top of 464416a2eaadf84d2bfdf795007863d03b222b7c >> (sb/packfiles-in-repository). >> It is also available at https://github.com/stefanbeller/git/tree/object-store-3 > > This series conflicts with 1731a1e239 (replace_object: convert struct > replace_object to object_id) and b383a13cc0 (Convert > lookup_replace_object to struct object_id), which are in next. ok, I'll investigate. Maybe the reroll will be on top of a merge of brians series and sb/packfiles-in-repository, both of which go to master quickly now that 2.17 is out? > >> This series will bring the replacement mechanism (git replace) >> into the object store. > > Good idea. heh, thanks. I think this is the smallest unit in which the community is willing to digest it. (The largest coherent patch series was the demo of "everything in struct repo"[1]) [1] https://public-inbox.org/git/20180205235508.216277-1-sbeller@xxxxxxxxxx/ > >> $ git diff 464416a2eaadf84d2bfdf795007863d03b222b7c..HEAD -- object-store.h repository.h >> diff --git a/object-store.h b/object-store.h >> index fef33f345f..be90c02db6 100644 >> --- a/object-store.h >> +++ b/object-store.h >> @@ -93,6 +93,22 @@ struct raw_object_store { >> struct alternate_object_database *alt_odb_list; >> struct alternate_object_database **alt_odb_tail; >> >> + /* >> + * Objects that should be substituted by other objects >> + * (see git-replace(1)). >> + */ >> + struct replace_objects { >> + /* >> + * An array of replacements. The array is kept sorted by the original >> + * sha1. >> + */ >> + struct replace_object **items; >> + >> + int alloc, nr; >> + >> + unsigned prepared : 1; >> + } replacements; > > An oidmap would be a better fit -- lookups should be quicker and > memory consumption not much worse. I meant to submit something like > this eventually after Brian's series lands: > > -- >8 -- > Subject: [PATCH] replace_object: use oidmap > > Load the replace objects into an oidmap to allow for easy lookups in > constant time. > > Signed-off-by: Rene Scharfe <l.s.r@xxxxxx> > --- > This is on top of next. So this is on top of brians series (modulo other next-ish stuff) which this series ought to merge with? The patch looks good -- just from the diff stat alone. Thanks, Stefan > > replace_object.c | 76 ++++++++++-------------------------------------- > 1 file changed, 16 insertions(+), 60 deletions(-) > > diff --git a/replace_object.c b/replace_object.c > index 336357394d..a757a5ebf2 100644 > --- a/replace_object.c > +++ b/replace_object.c > @@ -1,54 +1,14 @@ > #include "cache.h" > -#include "sha1-lookup.h" > +#include "oidmap.h" > #include "refs.h" > #include "commit.h" > > -/* > - * An array of replacements. The array is kept sorted by the original > - * sha1. > - */ > -static struct replace_object { > - struct object_id original; > +struct replace_object { > + struct oidmap_entry original; > struct object_id replacement; > -} **replace_object; > - > -static int replace_object_alloc, replace_object_nr; > +}; > > -static const unsigned char *replace_sha1_access(size_t index, void *table) > -{ > - struct replace_object **replace = table; > - return replace[index]->original.hash; > -} > - > -static int replace_object_pos(const unsigned char *sha1) > -{ > - return sha1_pos(sha1, replace_object, replace_object_nr, > - replace_sha1_access); > -} > - > -static int register_replace_object(struct replace_object *replace, > - int ignore_dups) > -{ > - int pos = replace_object_pos(replace->original.hash); > - > - if (0 <= pos) { > - if (ignore_dups) > - free(replace); > - else { > - free(replace_object[pos]); > - replace_object[pos] = replace; > - } > - return 1; > - } > - pos = -pos - 1; > - ALLOC_GROW(replace_object, replace_object_nr + 1, replace_object_alloc); > - replace_object_nr++; > - if (pos < replace_object_nr) > - MOVE_ARRAY(replace_object + pos + 1, replace_object + pos, > - replace_object_nr - pos - 1); > - replace_object[pos] = replace; > - return 0; > -} > +static struct oidmap replace_map = OIDMAP_INIT; > > static int register_replace_ref(const char *refname, > const struct object_id *oid, > @@ -59,7 +19,7 @@ static int register_replace_ref(const char *refname, > const char *hash = slash ? slash + 1 : refname; > struct replace_object *repl_obj = xmalloc(sizeof(*repl_obj)); > > - if (get_oid_hex(hash, &repl_obj->original)) { > + if (get_oid_hex(hash, &repl_obj->original.oid)) { > free(repl_obj); > warning("bad replace ref name: %s", refname); > return 0; > @@ -69,7 +29,7 @@ static int register_replace_ref(const char *refname, > oidcpy(&repl_obj->replacement, oid); > > /* Register new object */ > - if (register_replace_object(repl_obj, 1)) > + if (oidmap_put(&replace_map, repl_obj)) > die("duplicate replace ref: %s", refname); > > return 0; > @@ -84,7 +44,7 @@ static void prepare_replace_object(void) > > for_each_replace_ref(register_replace_ref, NULL); > replace_object_prepared = 1; > - if (!replace_object_nr) > + if (!replace_map.map.tablesize) > check_replace_refs = 0; > } > > @@ -100,21 +60,17 @@ static void prepare_replace_object(void) > */ > const struct object_id *do_lookup_replace_object(const struct object_id *oid) > { > - int pos, depth = MAXREPLACEDEPTH; > + int depth = MAXREPLACEDEPTH; > const struct object_id *cur = oid; > > prepare_replace_object(); > > /* Try to recursively replace the object */ > - do { > - if (--depth < 0) > - die("replace depth too high for object %s", > - oid_to_hex(oid)); > - > - pos = replace_object_pos(cur->hash); > - if (0 <= pos) > - cur = &replace_object[pos]->replacement; > - } while (0 <= pos); > - > - return cur; > + while (depth-- > 0) { > + struct replace_object *repl_obj = oidmap_get(&replace_map, cur); > + if (!repl_obj) > + return cur; > + cur = &repl_obj->replacement; > + } > + die("replace depth too high for object %s", oid_to_hex(oid)); > } > -- > 2.17.0
