On 04/18/2015 02:40 AM, Junio C Hamano wrote:
Jeff King <peff@xxxxxxxx> writes:
On Fri, Apr 17, 2015 at 09:21:31AM -0700, Junio C Hamano wrote:
Jeff King <peff@xxxxxxxx> writes:
If there _is_ a performance implication to worry about here, I think it
would be that we are doing an extra malloc/free.
Thanks for reminding me; yes, that also worried me.
As an aside, I worried about the extra allocation for reading the header
in the first place. But it looks like we only do this on the --literally
code path (and otherwise use the normal unpack_sha1_header). Still, I
wonder if we could make this work automagically. That is, speculatively
unpack the first N bytes, assuming we hit the end-of-header. If not,
then go to a strbuf as the slow path. Then it would be fine to cover all
cases; the normal ones would be fast, and only ridiculous things would
incur the extra allocation.
Yes, that was what I was hoping to see eventually ;-)
Sorry for the delay, So after reading what Jeff said I tried to
implement it, this might not be a final version of the change, more of a
RFC version. What do you'll think?
@@ -1564,6 +1564,36 @@ int unpack_sha1_header(git_zstream *stream,
unsigned char *map, unsigned long ma
return git_inflate(stream, 0);
}
+static int unpack_sha1_header_to_strbuf(git_zstream *stream, unsigned
char *map,
+ unsigned long mapsize, void *buffer,
+ unsigned long bufsiz, struct
strbuf *header)
+{
+ unsigned char *cp;
+ int status;
+ int i = 0;
+
+ status = unpack_sha1_header(stream, map, mapsize, buffer, bufsiz);
+
+ for (cp = buffer; cp < stream->next_out; cp++)
+ if (!*cp) {
+ /* Found the NUL at the end of the header */
+ return 0;
+ }
+
+ strbuf_add(header, buffer, stream->next_out - (unsigned char
*)buffer);
+ do {
+ status = git_inflate(stream, 0);
+ strbuf_add(header, buffer, stream->next_out - (unsigned
char *)buffer);
+ for (cp = buffer; cp < stream->next_out; cp++)
+ if (!*cp)
+ /* Found the NUL at the end of the header */
+ return 0;
+ stream->next_out = buffer;
+ stream->avail_out = bufsiz;
+ } while (status != Z_STREAM_END);
+ return -1;
+}
+
@@ -2555,17 +2610,24 @@ static int sha1_loose_object_info(const unsigned
char *sha1,
return -1;
if (oi->disk_sizep)
*oi->disk_sizep = mapsize;
- if (unpack_sha1_header(&stream, map, mapsize, hdr, sizeof(hdr)) < 0)
+ if ((flags & LOOKUP_LITERALLY)) {
+ if (unpack_sha1_header_to_strbuf(&stream, map, mapsize,
hdr, sizeof(hdr), &hdrbuf) < 0)
+ status = error("unable to unpack %s header with
--literally",
+ sha1_to_hex(sha1));
+ } else if (unpack_sha1_header(&stream, map, mapsize, hdr,
sizeof(hdr)) < 0)
status = error("unable to unpack %s header",
sha1_to_hex(sha1));
- else if ((status = parse_sha1_header(hdr, &size)) < 0)
+ if (hdrbuf.len) {
+ if ((status = parse_sha1_header_extended(hdrbuf.buf, oi,
flags)) < 0)
+ status = error("unable to parse %s header with
--literally",
+ sha1_to_hex(sha1));
+ } else if ((status = parse_sha1_header_extended(hdr, oi, flags))
< 0)
status = error("unable to parse %s header",
sha1_to_hex(sha1));
and
--
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