On Mon, Jun 29, 2015 at 11:10 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Karthik Nayak <karthik.188@xxxxxxxxx> writes:
>
>> +/*
>> + * Given a ref (sha1, refname) see if it points to one of the sha1s
>> + * in a sha1_array.
>> + */
>> +static int match_points_at(struct sha1_array *points_at, const unsigned char *sha1,
>> + const char *refname)
>> +{
>> + struct object *obj;
>> +
>> + if (!points_at || !points_at->nr)
>> + return 1;
>> +
>> + if (sha1_array_lookup(points_at, sha1) >= 0)
>> + return 1;
>> +
>> + obj = parse_object_or_die(sha1, refname);
>> + if (obj->type == OBJ_TAG &&
>> + sha1_array_lookup(points_at, ((struct tag *)obj)->tagged->sha1) >= 0)
>> + return 1;
>> +
>> + return 0;
>> +}
>> +
>
> Interesting. I think the change done while copying the code does
> not change anything from the original (other than that the helper
> lost its ability to return the peeled object name), and I think you
> shouldn't make any change while copying the code that would change
> the benaviour, but I notice a few things that we might want to keep
> in mind and revisit them later (i.e. might be a good idea to add
> NEEDSWORK comment to record them near the function):
Reverted the change.
OK will do and add this, I will work on this after GSoC is done.
But like you said I'll add a comment so if someone wants to they can
work on it for now.
>
> - The original only peeled one level of indirection, so does this
> implementation. But is that really what we want, I wonder?
>
> After doing:
>
> $ git tag -a -m 'annotated' atag $commit
> $ git tag -a -m 'annotated doubly' dtag atag
>
> atag^0, dtag^0 and $commit all refer to the same commit object.
> Do we want to miss dtag with --point-at=$commit?
>
> - As we are in for-each-ref (or eventually tag -l) that is walking
> the cached refs, we may know what refname peels to without
> parsing the object at all. Could it be more efficient to ask
> peel_ref() for the pointee without doing parse_object()
> ourselves?
>
Shouldn't both these scenarios be solved together by using peel_ref()?
After what you said I just tried a hacked up version of using peel_ref()
rather than parsing the object, tried it out on the Linux tree.
$time git tag -l --points-at=HEAD~501
git tag -l --points-at=HEAD~501 0.03s user 0.01s system 97% cpu 0.044 total
*Using the modified version which uses peel_ref() *
$time ../git/git tag -l --points-at=HEAD~501
../git/git tag -l --points-at=HEAD~501 0.01s user 0.02s system 90%
cpu 0.033 total
This was the average of around 5 tests, Might not be the best way to check, but
I'm sure there's an improvement.
Thanks :)
--
Regards,
Karthik Nayak
--
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