Re: [PATCH 3/3] refs: use strings directly in find_containing_dir()

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

 



On 05/23/2012 07:15 PM, René Scharfe wrote:
Am 23.05.2012 18:56, schrieb Junio C Hamano:
René Scharfe<rene.scharfe@xxxxxxxxxxxxxx>   writes:

Am 23.05.2012 00:18, schrieb Junio C Hamano:
René Scharfe<rene.scharfe@xxxxxxxxxxxxxx>    writes:

What has git grep to do with refs?  It checks if the path in the command
above is a ref, which makes it iterate over all of them..

Do you mean:

	/* Is it a rev? */
           get_sha1()
           ->    ...
             ->    get_sha1_basic()
               ->    dwim_ref()

callpath?

Yes, indeed.  Hmm, this is done even if the paths come after a
double-dash.  Anyway, I don't consider the check to be a performance
issue, just a quick way to test the allocation count that i stumbled
upon while working on the recent grep patches.

I was merely reacting "iterate over all of them"; dwim_ref() only checks
if .git/blah, .git/refs/heads/blah, .git/refs/tags/blah, etc.  exists and
the number of checks do not depend on the number of refs you have, so I
was wondering if I overlooked something that does for_each_ref() of
everything.

Yeah, for loose refs that's true. However, I have 470 packed refs, and
this command:

	$ valgrind --tool=exp-dhat ./git grep guess xdiff/xutils.c

reports (among other findings):

==28255== max-live:    30,334 in 470 blocks
==28255== tot-alloc:   30,334 in 470 blocks (avg size 64.54)
==28255== deaths:      none (none of these blocks were freed)
==28255== acc-ratios:  7.76 rd, 0.95 wr  (235,582 b-read, 28,924 b-written)
==28255==    at 0x402AEE8: malloc (in /usr/lib/valgrind/vgpreload_exp-dhat-x86-linux.so)
==28255==    by 0x813691D: xmalloc (wrapper.c:50)
==28255==    by 0x8106B1A: create_ref_entry.constprop.8 (refs.c:250)
==28255==    by 0x8107761: read_packed_refs (refs.c:817)
==28255==    by 0x810785F: get_packed_refs (refs.c:843)
==28255==    by 0x8107BE7: resolve_ref_unsafe (refs.c:1028)
==28255==    by 0x81090AA: dwim_ref (refs.c:1549)
==28255==    by 0x8122E06: get_sha1_1 (sha1_name.c:304)
==28255==    by 0x81237EF: get_sha1_with_context_1 (sha1_name.c:1044)
==28255==    by 0x8124016: get_sha1 (cache.h:795)
==28255==    by 0x75782F65: ???

You are seeing all of the packed refs being *read*, which is triggered in this case by resolve_ref_unsafe() -> get_packed_refs(). This stack trace does not imply that all references are being iterated through as part of a search. IOW, if dwim_ref() were called again, there would not be another 470 allocations.

This is similar to the old code, which also read all of the packed refs whenever any of them were accessed.

Michael

--
Michael Haggerty
mhagger@xxxxxxxxxxxx
http://softwareswirl.blogspot.com/
--
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]