]> git.ipfire.org Git - thirdparty/git.git/commit - pack-bitmap.c
pack-objects: drop packlist index_pos optimization
authorJeff King <peff@peff.net>
Fri, 6 Sep 2019 01:36:05 +0000 (21:36 -0400)
committerJunio C Hamano <gitster@pobox.com>
Fri, 6 Sep 2019 18:03:42 +0000 (11:03 -0700)
commit3a37876b5dca4c18bda67bcdead9c1d79a59933d
tree56e4477811ed88a2510d65acc3eb33755563825e
parent7e6b96c73b24a797762440e7a1ef0eaf5e7a8dca
pack-objects: drop packlist index_pos optimization

Once upon a time, the code to add an object to our packing list in
pack-objects all lived in a single function. It computed the position
within the hash table once, then used it to check if the object was
already present, and if not, to add it.

Later, in 2834bc27c1 (pack-objects: refactor the packing list,
2013-10-24), this was split into two functions: packlist_find() and
packlist_alloc(). We ended up with an "index_pos" variable that gets
passed through several functions to make it from one to the other.

The resulting code is rather confusing to follow. The "index_pos"
variable is sometimes undefined, if we don't yet have a hash table. This
works out in practice because in that case packlist_alloc() won't use it
at all, since it will have to create/grow the hash table. But it's hard
to verify that, and it does cause gcc 9.2.1's -Wmaybe-uninitialized to
complain when compiled with "-flto -O3" (rightfully, since we do pass
the uninitialized value as a function parameter, even if nobody ends up
using it).

All of this is to save computing the hash index again when we're
inserting into the hash table, which I found doesn't make a measurable
difference in the program runtime (which is not surprising, since we're
doing all kinds of other heavyweight things for each object).

Let's just drop this index_pos variable entirely, simplifying the code
(and pleasing the compiler).

We might be better still refactoring this custom hash table to use one
of our existing implementations (an oidmap, or a kh_oid_map). I stopped
short of that here, but this would be the likely first step towards that
anyway.

Reported-by: Stephan Beyer <s-beyer@gmx.net>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
builtin/pack-objects.c
pack-bitmap-write.c
pack-bitmap.c
pack-objects.c
pack-objects.h