| Message ID | b87e7e01-baa6-6fb2-7081-0042ecd3b6b7@web.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] object-store: factor out odb_loose_cache() | expand |
On Sun, Jan 06, 2019 at 05:45:30PM +0100, René Scharfe wrote: > Add and use a function for loading the entries if a loose object > subdirectory for a given object ID. It frees callers from deriving the > fanout key; they can use the returned oid_array reference for lookups or > forward range scans. Much nicer. > diff --git a/object-store.h b/object-store.h > index 60758efad8..7236c571c0 100644 > --- a/object-store.h > +++ b/object-store.h > @@ -54,6 +54,13 @@ void add_to_alternates_memory(const char *dir); > */ > void odb_load_loose_cache(struct object_directory *odb, int subdir_nr); > > +/* > + * Populate and return the loose object cache array corresponding to the > + * given object ID. > + */ > +struct oid_array *odb_loose_cache(struct object_directory *odb, > + const struct object_id *oid); > + I think the ugly-interfaced odb_load_loose_cache() can become "static" now, as the only outside caller (from sha1-name.c) has gone away. > +struct oid_array *odb_loose_cache(struct object_directory *odb, > + const struct object_id *oid) > +{ > + int subdir_nr = oid->hash[0]; > + odb_load_loose_cache(odb, subdir_nr); > + return &odb->loose_objects_cache; > +} > + > void odb_load_loose_cache(struct object_directory *odb, int subdir_nr) You'd need to re-order these definitions, of course (or alternatively, just fold the load function inline into odb_loose_cache()). -Peff
On 06/01/2019 16:45, René Scharfe wrote: > Add and use a function for loading the entries if a loose object > subdirectory for a given object ID. The second part of the sentence 'a loose object subdirectory for a given object ID' does not scan for me. Is there a word missing? > It frees callers from deriving the > fanout key; they can use the returned oid_array reference for lookups or > forward range scans. > > Suggested-by: Jeff King<peff@peff.net> > Signed-off-by: Rene Scharfe<l.s.r@web.de>
On Mon, Jan 07, 2019 at 11:27:06AM +0000, Philip Oakley wrote: > On 06/01/2019 16:45, René Scharfe wrote: > > Add and use a function for loading the entries if a loose object > > subdirectory for a given object ID. > > The second part of the sentence 'a loose object subdirectory for a given > object ID' does not scan for me. Is there a word missing? I think s/if/in/ ? -Peff
Am 07.01.2019 um 13:30 schrieb Jeff King: > On Mon, Jan 07, 2019 at 11:27:06AM +0000, Philip Oakley wrote: > >> On 06/01/2019 16:45, René Scharfe wrote: >>> Add and use a function for loading the entries if a loose object >>> subdirectory for a given object ID. >> >> The second part of the sentence 'a loose object subdirectory for a given >> object ID' does not scan for me. Is there a word missing? > > I think s/if/in/ ? Good guess and close, but I think I meant s/if/of/. Anyway, the idea is that a caller provides an object ID and the function then reads the corresponding object subdirectory. René
Am 07.01.2019 um 09:27 schrieb Jeff King: > On Sun, Jan 06, 2019 at 05:45:30PM +0100, René Scharfe wrote: >> diff --git a/object-store.h b/object-store.h >> index 60758efad8..7236c571c0 100644 >> --- a/object-store.h >> +++ b/object-store.h >> @@ -54,6 +54,13 @@ void add_to_alternates_memory(const char *dir); >> */ >> void odb_load_loose_cache(struct object_directory *odb, int subdir_nr); >> >> +/* >> + * Populate and return the loose object cache array corresponding to the >> + * given object ID. >> + */ >> +struct oid_array *odb_loose_cache(struct object_directory *odb, >> + const struct object_id *oid); >> + > > I think the ugly-interfaced odb_load_loose_cache() can become "static" > now, as the only outside caller (from sha1-name.c) has gone away. > >> +struct oid_array *odb_loose_cache(struct object_directory *odb, >> + const struct object_id *oid) >> +{ >> + int subdir_nr = oid->hash[0]; >> + odb_load_loose_cache(odb, subdir_nr); >> + return &odb->loose_objects_cache; >> +} >> + >> void odb_load_loose_cache(struct object_directory *odb, int subdir_nr) > > You'd need to re-order these definitions, of course (or alternatively, > just fold the load function inline into odb_loose_cache()). Yes, the functions are arranged so that odb_load_loose_cache() can be inlined easily. I meant to include a patch for that but then quibbled about keeping the BUG check (which is probably optimized out) or not, and dropped it for now to get the performance fix in more quickly.. René
Am 07.01.2019 um 14:26 schrieb René Scharfe: > Yes, the functions are arranged so that odb_load_loose_cache() can be > inlined easily. I meant to include a patch for that but then quibbled > about keeping the BUG check (which is probably optimized out) or not, > and dropped it for now to get the performance fix in more quickly.. ... which is probably just my laziness talking. Sent a minimal patch for that now. René
diff --git a/object-store.h b/object-store.h index 60758efad8..7236c571c0 100644 --- a/object-store.h +++ b/object-store.h @@ -54,6 +54,13 @@ void add_to_alternates_memory(const char *dir); */ void odb_load_loose_cache(struct object_directory *odb, int subdir_nr); +/* + * Populate and return the loose object cache array corresponding to the + * given object ID. + */ +struct oid_array *odb_loose_cache(struct object_directory *odb, + const struct object_id *oid); + struct packed_git { struct packed_git *next; struct list_head mru; diff --git a/sha1-file.c b/sha1-file.c index 5a272f70de..cb8583b634 100644 --- a/sha1-file.c +++ b/sha1-file.c @@ -924,7 +924,6 @@ static int open_sha1_file(struct repository *r, static int quick_has_loose(struct repository *r, const unsigned char *sha1) { - int subdir_nr = sha1[0]; struct object_id oid; struct object_directory *odb; @@ -932,8 +931,7 @@ static int quick_has_loose(struct repository *r, prepare_alt_odb(r); for (odb = r->objects->odb; odb; odb = odb->next) { - odb_load_loose_cache(odb, subdir_nr); - if (oid_array_lookup(&odb->loose_objects_cache, &oid) >= 0) + if (oid_array_lookup(odb_loose_cache(odb, &oid), &oid) >= 0) return 1; } return 0; @@ -2152,6 +2150,14 @@ static int append_loose_object(const struct object_id *oid, const char *path, return 0; } +struct oid_array *odb_loose_cache(struct object_directory *odb, + const struct object_id *oid) +{ + int subdir_nr = oid->hash[0]; + odb_load_loose_cache(odb, subdir_nr); + return &odb->loose_objects_cache; +} + void odb_load_loose_cache(struct object_directory *odb, int subdir_nr) { struct strbuf buf = STRBUF_INIT; diff --git a/sha1-name.c b/sha1-name.c index b24502811b..a656481c6a 100644 --- a/sha1-name.c +++ b/sha1-name.c @@ -87,21 +87,21 @@ static int match_sha(unsigned, const unsigned char *, const unsigned char *); static void find_short_object_filename(struct disambiguate_state *ds) { - int subdir_nr = ds->bin_pfx.hash[0]; struct object_directory *odb; for (odb = the_repository->objects->odb; odb && !ds->ambiguous; odb = odb->next) { int pos; + struct oid_array *loose_objects; - odb_load_loose_cache(odb, subdir_nr); - pos = oid_array_lookup(&odb->loose_objects_cache, &ds->bin_pfx); + loose_objects = odb_loose_cache(odb, &ds->bin_pfx); + pos = oid_array_lookup(loose_objects, &ds->bin_pfx); if (pos < 0) pos = -1 - pos; - while (!ds->ambiguous && pos < odb->loose_objects_cache.nr) { + while (!ds->ambiguous && pos < loose_objects->nr) { const struct object_id *oid; - oid = odb->loose_objects_cache.oid + pos; + oid = loose_objects->oid + pos; if (!match_sha(ds->len, ds->bin_pfx.hash, oid->hash)) break; update_candidates(ds, oid);
Add and use a function for loading the entries if a loose object subdirectory for a given object ID. It frees callers from deriving the fanout key; they can use the returned oid_array reference for lookups or forward range scans. Suggested-by: Jeff King <peff@peff.net> Signed-off-by: Rene Scharfe <l.s.r@web.de> --- object-store.h | 7 +++++++ sha1-file.c | 12 +++++++++--- sha1-name.c | 10 +++++----- 3 files changed, 21 insertions(+), 8 deletions(-)