Skip to content

Commit a650ad9

Browse files
committed
odb: do not use "blank" substitute for NULL
When various *object_info() functions are given an extended object info structure as NULL by a caller that does not want any details, the code uses a file-scope static blank_oi and passes it down to the helper functions they use, to avoid handling NULL specifically. The ps/object-read-stream topic graduated to 'master' recently however had a bug that assumed that two identically named file-scope static variables in two functions are the same, which of course is not the case. This made "git commit" take 0.38 seconds to 1508 seconds in some case, as reported by Aaron Plattner here: https://lore.kernel.org/git/f4ba7e89-4717-4b36-921f-56537131fd69@nvidia.com/ We _could_ move the blank_oi variable to the global scope in common section to fix this regression, but explicitly handling the NULL is a much safer fix. It would also reduce the chance of errors that somebody accidentally writes into blank_oi, making its contents dirty, which potentially will make subsequent calls into the function misbehave. By explicitly handling NULL input, we no longer have to worry about it. Reported-by: Aaron Plattner <aplattner@nvidia.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent 1da2a42 commit a650ad9

3 files changed

Lines changed: 18 additions & 22 deletions

File tree

object-file.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -426,7 +426,7 @@ int odb_source_loose_read_object_info(struct odb_source *source,
426426
unsigned long size_scratch;
427427
enum object_type type_scratch;
428428

429-
if (oi->delta_base_oid)
429+
if (oi && oi->delta_base_oid)
430430
oidclr(oi->delta_base_oid, source->odb->repo->hash_algo);
431431

432432
/*
@@ -437,13 +437,13 @@ int odb_source_loose_read_object_info(struct odb_source *source,
437437
* return value implicitly indicates whether the
438438
* object even exists.
439439
*/
440-
if (!oi->typep && !oi->sizep && !oi->contentp) {
440+
if (!oi || (!oi->typep && !oi->sizep && !oi->contentp)) {
441441
struct stat st;
442-
if (!oi->disk_sizep && (flags & OBJECT_INFO_QUICK))
442+
if ((!oi || !oi->disk_sizep) && (flags & OBJECT_INFO_QUICK))
443443
return quick_has_loose(source->loose, oid) ? 0 : -1;
444444
if (stat_loose_object(source->loose, oid, &st, &path) < 0)
445445
return -1;
446-
if (oi->disk_sizep)
446+
if (oi && oi->disk_sizep)
447447
*oi->disk_sizep = st.st_size;
448448
return 0;
449449
}

odb.c

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -664,34 +664,31 @@ static int do_oid_object_info_extended(struct object_database *odb,
664664
const struct object_id *oid,
665665
struct object_info *oi, unsigned flags)
666666
{
667-
static struct object_info blank_oi = OBJECT_INFO_INIT;
668667
const struct cached_object *co;
669668
const struct object_id *real = oid;
670669
int already_retried = 0;
671670

672-
673671
if (flags & OBJECT_INFO_LOOKUP_REPLACE)
674672
real = lookup_replace_object(odb->repo, oid);
675673

676674
if (is_null_oid(real))
677675
return -1;
678676

679-
if (!oi)
680-
oi = &blank_oi;
681-
682677
co = find_cached_object(odb, real);
683678
if (co) {
684-
if (oi->typep)
685-
*(oi->typep) = co->type;
686-
if (oi->sizep)
687-
*(oi->sizep) = co->size;
688-
if (oi->disk_sizep)
689-
*(oi->disk_sizep) = 0;
690-
if (oi->delta_base_oid)
691-
oidclr(oi->delta_base_oid, odb->repo->hash_algo);
692-
if (oi->contentp)
693-
*oi->contentp = xmemdupz(co->buf, co->size);
694-
oi->whence = OI_CACHED;
679+
if (oi) {
680+
if (oi->typep)
681+
*(oi->typep) = co->type;
682+
if (oi->sizep)
683+
*(oi->sizep) = co->size;
684+
if (oi->disk_sizep)
685+
*(oi->disk_sizep) = 0;
686+
if (oi->delta_base_oid)
687+
oidclr(oi->delta_base_oid, odb->repo->hash_algo);
688+
if (oi->contentp)
689+
*oi->contentp = xmemdupz(co->buf, co->size);
690+
oi->whence = OI_CACHED;
691+
}
695692
return 0;
696693
}
697694

packfile.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2095,7 +2095,6 @@ int packfile_store_read_object_info(struct packfile_store *store,
20952095
struct object_info *oi,
20962096
unsigned flags UNUSED)
20972097
{
2098-
static struct object_info blank_oi = OBJECT_INFO_INIT;
20992098
struct pack_entry e;
21002099
int rtype;
21012100

@@ -2106,7 +2105,7 @@ int packfile_store_read_object_info(struct packfile_store *store,
21062105
* We know that the caller doesn't actually need the
21072106
* information below, so return early.
21082107
*/
2109-
if (oi == &blank_oi)
2108+
if (!oi)
21102109
return 0;
21112110

21122111
rtype = packed_object_info(store->odb->repo, e.p, e.offset, oi);

0 commit comments

Comments
 (0)