Re: [PATCH v8 1/4] sha1_file.c: support reading from a loose object of unknown type

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

 





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




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