[PATCH 1/4] peel_ref: use faster deref_tag_noverify

2012-10-04 Thread Jeff King
When we are asked to peel a ref to a sha1, we internally call
deref_tag, which will recursively parse each tagged object
until we reach a non-tag. This has the benefit that we will
verify our ability to load and parse the pointed-to object.

However, there is a performance downside: we may not need to
load that object at all (e.g., if we are listing peeled
simply listing peeled refs), or it may be a large object
that should follow a streaming code path (e.g., an annotated
tag of a large blob).

It makes more sense for peel_ref to choose the fast thing
rather than performing the extra check, for two reasons:

  1. We will already sometimes short-circuit the tag parsing
 in favor of a peeled entry from a packed-refs file. So
 we are already favoring speed in some cases, and it is
 not wise for a caller to rely on peel_ref to detect
 corruption.

  2. We already silently ignore much larger corruptions,
 like a ref that points to a non-existent object, or a
 tag object that exists but is corrupted.

  2. peel_ref is not the right place to check for such a
 database corruption. It is returning only the sha1
 anyway, not the actual object. Any callers which use
 that sha1 to load an object will soon discover the
 corruption anyway, so we are really just pushing back
 the discovery to later in the program.

Signed-off-by: Jeff King p...@peff.net
---
This is basically bringing the optimization from 90108a2 (upload-pack:
avoid parsing tag destinations, 2012-01-06) to users of peel_ref.

 refs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/refs.c b/refs.c
index da74a2b..0a916a0 100644
--- a/refs.c
+++ b/refs.c
@@ -1225,7 +1225,7 @@ fallback:
 fallback:
o = parse_object(base);
if (o  o-type == OBJ_TAG) {
-   o = deref_tag(o, refname, 0);
+   o = deref_tag_noverify(o);
if (o) {
hashcpy(sha1, o-sha1);
return 0;
-- 
1.8.0.rc0.10.g8dd2a92

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


Re: [PATCH 1/4] peel_ref: use faster deref_tag_noverify

2012-10-04 Thread Junio C Hamano
Jeff King p...@peff.net writes:

 When we are asked to peel a ref to a sha1, we internally call
 deref_tag, which will recursively parse each tagged object
 until we reach a non-tag. This has the benefit that we will
 verify our ability to load and parse the pointed-to object.

 However, there is a performance downside: we may not need to
 load that object at all (e.g., if we are listing peeled
 simply listing peeled refs),...

Both correct.

I checked the existing callsites and nobody seems to assume that it
can use the pointed-to object without checking because peel_ref has
already checked, so it makes sense.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html