Re: [PATCH v2 2/3] fast-export: improve speed by skipping blobs

From: Jeff King <peff@peff.net>
Date: May 6, 2013, 12:31 UTC
Message-ID: <20130506123111.GB3809@sigill.intra.peff.net>
In-Reply-To: <1367793534-8401-3-git-send-email-felipe.contreras@gmail.com>

On Sun, May 05, 2013 at 05:38:53PM -0500, Felipe Contreras wrote:

> We don't care about blobs, or any object other than commits, but in
> order to find the type of object, we are parsing the whole thing, which
> is slow, specially in big repositories with lots of big files.

I did a double-take on reading this subject line and first paragraph, thinking "surely fast-export needs to actually output blobs?".

Reading the patch, I see that this is only about not bothering to load blob marks from --import-marks. It might be nice to mention that in the commit message, which is otherwise quite confusing.

I'm also not sure why your claim "we don't care about blobs" is true, because naively we would want future runs of fast-export to avoid having to write out the whole blob content when mentioning the blob again. I think one argument could be "if we write a mark for blob X, we will also have written a mark for commit Y which contains it; on subsequent runs, we will just show the mark for Y in the first place, and not even care about showing X (as a part of Y) either way. We would only refer to the mark for X if it appears as part of a different commit, but that is a rare case not worth worrying about."

Does that match your reasoning?

> Before this, loading the objects of a fresh emacs import, with 260598
> blobs took 14 minutes, after this patch, it takes 3 seconds.

Presumably most of that speed improvement comes from not parsing the blob objects. I wonder if you could get similar speedups by applying the "do not bother parsing" rule from your patch 3. You would still incur some cost to create a "struct blob", but it may or may not be measurable. That would mean we get the "case not worth worrying about" from above for free. I doubt it would make that big a difference, though, given the rarity of it. So I am OK with it either way.

-Peff