Re: [PATCH 00/11] writing out a huge blob to working tree
- From
Jeff King <peff@peff.net>
- Date
- May 18, 2011, 08:17 UTC
- Message-ID
- <20110518081733.GF27482@sigill.intra.peff.net>
- In-Reply-To
- <1305505831-31587-1-git-send-email-gitster@pobox.com>
On Sun, May 15, 2011 at 05:30:20PM -0700, Junio C Hamano wrote:
> Recently "diff" learned to avoid reading the contents only to say "Binary > files differ" when these large blobs are marked as binary.
With your series, we should be able to get similar speedups even if the user didn't explicitly mark a file as binary. We need only peek at the beginning of a blob to see if it is binary, so we can be conservative with big files. Something like this (which doesn't work because of the "size" bug I mentioned elsewhere, but is meant to be illustrative):
diff --git a/diff.c b/diff.c index ba5f7aa..bfe1b2d 100644 --- a/diff.c +++ b/diff.c @@ -15,6 +15,7 @@ #include "sigchain.h" #include "submodule.h" #include "ll-merge.h" +#include "streaming.h" #ifdef NO_FAST_WORKING_DIRECTORY #define FAST_WORKING_DIRECTORY 0 @@ -1931,6 +1932,37 @@ static void diff_filespec_load_driver(struct diff_filespec *one) one->driver = userdiff_find_by_name("default"); } +static char *populate_or_peek(struct diff_filespec *df, + unsigned long want, + unsigned long *got) +{ + struct git_istream *st; + enum object_type type; + char *buf; + + st = open_istream(df->sha1, &type, &df->size); + if (!st) { + diff_populate_filespec(df, 0); + *got = df->size; + return df->data; + } + + if (df->size < big_file_threshold) { + buf = df->data = xmallocz(df->size); + want = df->size; + df->should_free = 1; + } + else + buf = xmallocz(want); + + /* looks like it will always read_in_full? */ + if (read_istream(st, buf, want) != want) + die("failed to read object"); + close_istream(st); + *got = want; + return buf; +} + int diff_filespec_is_binary(struct diff_filespec *one) { if (one->is_binary == -1) { @@ -1938,13 +1970,25 @@ int diff_filespec_is_binary(struct diff_filespec *one) if (one->driver->binary != -1) one->is_binary = one->driver->binary; else { - if (!one->data && DIFF_FILE_VALID(one)) - diff_populate_filespec(one, 0); - if (one->data) - one->is_binary = buffer_is_binary(one->data, - one->size); + char *buf; + unsigned long size; + + if (one->data) { + buf = one->data; + size = one->size; + } + else if (DIFF_FILE_VALID(one)) + buf = populate_or_peek(one, 8192, &size); + else + buf = NULL; + + if (buf) + one->is_binary = buffer_is_binary(buf, size); if (one->is_binary == -1) one->is_binary = 0; + + if (buf != one->data) + free(buf); } } return one->is_binary; I think a "peek" function like this would be a nice addition to the streaming API. Something like: char *peek_sha1(const unsigned char sha1[20], /* which object */ enum object_type *type, /* out: type */ unsigned long want, /* how much do we need */ unsigned long big, /* if less than this, just give us everything in the name of efficiency */ unsigned long *got, /* out: how much did we peek */ unsigned long *size, /* out: how big is the whole thing */ ); but maybe diff is the only place where that is useful. I dunno. -Peff