Re: [GSoC] Git Blog 12

On Tue, Aug 10, 2021 at 4:20 PM ZheNing Hu [off-list ref] wrote:

Christian Couder [off-list ref] 于2021年8月10日周二 下午4:04写道：

quoted

quoted

parse_object_buffer(), let's take a look at the result of gprof again:

We need to call grab_sub_body_contents(), grab_person() to rescan the
buffer and extract the data.
What if we can combine these multiple scanning and parsing into one completion?
At least intuitively, this has an opportunity to improve performance.

Yeah, but is there a way to check that we indeed scan or parse the
same objects multiple times? This way we might get an idea about how
much scanning and parsing we could save.

I think find_subpos() called by grab_sub_body_contents() and find_wholine()
called by grab_person() are evidences that we are repeating iteratively.
But the proportion of time they occupy is too small. 0.0142% and 0.0109%

Could adding traces help with measuring how many times we call
functions like get_object() or format_ref_array_item() for each
object?

I think it's interesting to have a better idea about how many times
functions that seem to take time like the above, or functions that
read objects, like oid_object_info_extended(), are called for each
object.

Sorry, but my attempts over the past two days have not gone well, the changes
here will make the program very complicated, the optimization here is not worth
doing.

Maybe but it would be interesting to document what didn't work and why.

quoted

quoted

So I check the implementation
details of `parse_commit_buffer()` and `parse_tag_buffer()`, maybe we
can pass some "hook pointer"
to these parsing functions like `oid_object_info_extended()` does to
extract only the information we need?

Would this also avoid scanning and parsing the same object many times?

oid_object_info_extended()? I think it can set the pointer and extract
the required
value. Well, the problem it solves may be a little different from here.

Could you explain a bit more?

I wonder if we sometimes call for example oid_object_info_extended()
once for an object and then parse_commit_buffer() for the same object,
when perhaps calling only parse_commit_buffer() once would have given
all the information we need.

quoted

quoted

I am thinking about whether it is possible to design a `struct
object_view` (temporarily called
`struct commit_view`) to store the offset of the parsed data in the
object content. `parse_commit_buffer()`
will check whether we need something for in-depth parsing. Like this:

struct commit_view {
int need_tree : 1;
int need_parents : 1;

int need_author : 1;
int need_author_name : 1;
int need_author_email : 1;
int need_author_date : 1;

int need_committer : 1;
int need_committer_name : 1;
int need_committer_email : 1;
int need_committer_date : 1;

Is the above info specific for each commit? Or will the above be the
same for all the commits we are processing?

According to the my previous thoughts, I think it is same for all commits.

Yeah, so I think it doesn't make sense to duplicate it for each
object. It could be a separate struct and we wouldn't need to have it
in a commit slab.

And I think maybe the above could be extended with fields like
"need_full_parsing" or "need_object_info" that could be computed from
the above fields. This could then help avoid several calls to
different functions when one could be enough.

Also aren't bitfields like the above "unsigned int" instead of "int"
in our code base?

quoted

Ok, so the idea is to use the commit slab feature to store the struct
commit_view instances. That seems reasonable to me.

It's a pity that it may not bring much optimization in real situations.

Maybe it's because copying data isn't actually a performance issue.

quoted

quoted

It seems that GSOC has only the last few weeks left, I'm not sure how
far this patch series is from
being merged by the master branch. Performance optimization may have
no end.

Yeah, but the idea for now is just to make using the ref-filter code
as fast as the current code.

It seems difficult to achieve consistent performance. As we discussed
before, the
previous `git cat-file --batch` can only provide a few kinds of
metadata that does not
need to be parsed, and after using ref-filter logic allows cat-file to
use more structured
information about git objects.

So the issue seems to be that we can't detect if only a few metadata
that don't need to be parsed are needed?

Could fields like "need_full_parsing" or "need_object_info" that I
suggest above help with that?

But this means it needs a harder path,
It requires many
traversals and many copies,

Could you explain why it needs many traversal and copies in more
details? And what could help avoid that?

If we really use the logic in ref-filter,
we can only do partial
optimization, we can't expect it to be as fast as the old function.
Unless we have the
opportunity to not use the logic in ref-filter, but use the new atoms
in ref-filter, this may
has a chance to escape the copy in the ref-filter. I don't know what
your opinion are...

Are you sure we couldn't detect in the ref-filter code that we need
only a few metadata that don't need to be parsed? And then skip
fetching and parsing data we don't need (without using a completely
separate code, like a fast path)?