Re: [GSOC] How to improve the performance of git cat-file --batch

From: Christian Couder <christian.couder@gmail.com>
Date: Jul 28, 2021, 15:36 UTC
Message-ID: <CAP8UFD1tS++xEMob7p5UeDQ_enoyy1aOQTG=28r-ge2k4n3ECw@mail.gmail.com>
In-Reply-To: <CAOLTT8QvtJ70X8mQx4K4gD0T=7i-ryd0QL81-QeSTqSWyHuWLQ@mail.gmail.com>

On Wed, Jul 28, 2021 at 3:38 PM ZheNing Hu <adlternative@gmail.com> wrote:

> Ok, therefore we need an accurate number of call times about lookup_object(),
> although the conclusion is obvious: 0 (upstream/master) and a big
> number (with my patch).

[...]

Show 7 quoted lines

> This is the only 1 time left is printed by git.c, which show that after using
> my patch, we additionally call  lookup_object() when we use --batch option.
> According to the results of the previous gprof test: lookup_object()
> occupies 8.72%
> of the total time. (Though below you seem to think that the effect of
> gprof is not
> reliable enough.) This may be a place worthy of optimization.

First, yeah, I hadn't seen the "calls" columns in your gprof reports. Sorry! It's nice to see that your manual check with a trace_printf() function gives the same result as gprof about this though.

Anyway if you agree that it might be a place worthy of optimization, then it might be a good idea to explain the reason for the numerous lookup_object() calls when using the ref-filter code.

[...]

Show 15 quoted lines

> > It would be nice if you could add a bit more details about how
> > lookup_object() is called (both before and after the changes that
> > degrade performance).
>
> After we letting git cat-file --batch reuse the logic of ref-filter,
> we will use get_object()
> to grab the object's data. Since we used atom %(raw), it will require
> us to grab the raw data
> of the object, oi->info.contentp will be set, parse_object_buffer() in
> get_object() will be
> called, parse_object_buffer() calls lookup_commit(), lookup_blob(),
> lookup_tree(),
> and lookup_tag(), they call lookup_object(). As we have seen,
> lookup_object() seems to
> take a lot of time.

Not sure why you are talking about %(raw). Is the root of the issue that we now use atom %(raw), or how we implemented it?

Show 12 quoted lines

> So let us think, can we skip this parse_object_buffer() in some scenarios?
> parse_object_buffer() parses the data of the object into a "struct
> object *obj", and then we use this
> obj feed to grab_values(), and then grab_values() feed obj to
> grab_tag_values() or grab_commit_values()
> to handle some logic about %(tag), %(type), %(object), %(tree),
> %(parent), %(numparent).
>
> But git cat-file --batch can avaid handle there atoms with default format.
>
> Therefore, maybe we can skip parsing object buffer if we really don't
> care about these atoms.

Yeah, maybe oi->info.contentp should be set only if the user specified one of the atoms that really needs the content provided by parse_object_buffer().

Thanks!