• Jeff King's avatar
    pack-objects: drop packlist index_pos optimization · 3a37876b
    Jeff King authored
    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 2834bc27 (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
    Reported-by: default avatarStephan Beyer <s-beyer@gmx.net>
    Signed-off-by: default avatarJeff King <peff@peff.net>
    Signed-off-by: default avatarJunio C Hamano <gitster@pobox.com>
pack-bitmap-write.c 12.7 KB