On Sat, Sep 14, 2013 at 4:26 AM, Thomas Rast <trast@xxxxxxxxxxx> wrote: > Junio C Hamano <gitster@xxxxxxxxx> writes: > >> Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> writes: >> >>> Current code makes pack-objects always do check_pack_crc() in >>> unpack_entry() even if right after that we find out there's a cached >>> version and pack access is not needed. Swap two code blocks, search >>> for cached version first, then check crc. > [...] >> >> Interesting. >> >> This is only triggered inside pack-objects, which would read a lot >> of data from existing packs, and the overhead for looking up the >> entry from the revindex, faulting in the actual packdata, and >> computing and comparing the crc would not be trivial, especially as >> the cost is incurred over many objects we need to untangle in the >> delta chain. If you have interesting numbers to show how much this >> improves the performance, I am curious to see it. No I don't have any timing numbers. I just updated the code to see how many times crc is checked and how many times we find a cached version after crc is checked. The numbers with git.git are 353535 and 113257 respectively. IOW we could reduce the number of crc checks by 30%. > > I can't see anything wrong with the patch, but then I haven't stared too > hard. (It seems that my conversion around abe601b (sha1_file: remove > recursion in unpack_entry, 2013-03-27) was faithful on this point, the > problem has existed for longer than that.) > > I tried the perf script below, but at least for the git repo the only > thing I can see is noise. --stdout does not set do_check_packed_object_crc, you need to run pack-objects without --stdout (i.e. the real use case is repack) > > --- 8< --- t/perf/p5300-pack-object.sh --- 8< --- > #!/bin/sh > > test_description="Tests object packing performance" > > . ./perf-lib.sh > > test_perf_default_repo > > test_perf 'pack-objects on commits in HEAD' ' > git rev-list HEAD | > git pack-objects --stdout >/dev/null > ' > > test_perf 'pack-objects on all of HEAD' ' > git rev-list --objects HEAD | > git pack-objects --stdout >/dev/null > ' > > test_done -- Duy -- 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