unpack_entry (was: [BUG] commit walk machinery is dangerous !)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <gitster@xxxxxxxxx> wrote:
> This is unrelated to the issue at hand, but I also notice that there are
> few callsites outside sha1_file.c that bypasses cache_or_unpack_entry()
> and call unpack_entry() directly.  I wonder if they should be using the
> cached version, making unpack_entry() static...

There are two such callsites that I found:

$ git grep -n --cached unpack_entry
fast-import.c:1201:     return unpack_entry(p, oe->offset, &type, sizep);
pack-check.c:114:               data = unpack_entry(p, entries[i].offset, &type,

Now the one in fast-import.c could be using the cached version.
This is simply fast-import trying to reuse sha1_file.c for reading
a previously written object to the pack.

Stuffing the item into the cache is perhaps pointless here as gfi
does its own sort of caching and only goes through this code path
when that caching has tossed the least-recently-used data and its
suddenly needed again.

So yea, it could be using the caching form, but it would maybe be
doing more work (and using more memory) than it really needs.


In pack-check.c we are looping through the objects in the order they
appear in the index so we can unpack them and verify each object's
SHA-1 signature.  Please note this is perhaps the slowest way to
enumerate through the pack and is why you can clone a repository
over git:// faster than you can run `fsck --full`.

If you really want to verify every single object's SHA-1, run the
damn pack through index-pack and compare the new index to the old
index (hint they should be identical, bit-for-bit).

So again, in this case the caching may only cause us to do more
work (and use more memory) than we need as we are slamming the
delta base cache with all sorts of unrelated base objects.  It
probably has a low hit ratio anyway during this loop.  :-|

-- 
Shawn.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux