Re: [PATCH] pack-objects: no crc check when the cached version is used

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

 



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




[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]