GSoC Week 12: Simplified version of parallel inflation

Patchset sent, with some limitations

Last week we had a prototype version of the parallel inflation patchset working without race conditions. Even so, it still had some major issues. For example:

• use_pack() needed an aditional provisory mutex to properly work in parallel;
• --textconv would produce considerable performance drops; and
• the code was a bit unorganized with some probably redundant locks.

These items would still take some time to fix. So my mentors and I thought it would be better to send a simplified version, for now, with the already stable improvements. And I could, then, keep working on a more complete version to be sent latter. This is good because is more incremental and, also, we can get community feedback sooner :)

So, this week, I’ve been working on this simplified version. It already brings good speedup to cached git-grep, but the optimization is disabled when --textconv or --recurse-submodules are used. A section was added to Documentation/git-grep.txt to better explain these details. I also repeated the performance tests I’ve been running, this time on an older machine. The final version can be seen here.

grep: don’t add submodules to alternates

While working at git-grep’s critical sections, I came to the following commentary at grep_submodules() (builtin/grep.c):

/*
* NEEDSWORK: This adds the submodule's object directory to the list of
* alternates for the single in-memory object store.  This has some bad
* consequences for memory (processed objects will never be freed) and
* performance (this increases the number of pack files git has to pay
* attention to, to the sum of the number of pack files in all the
* repositories processed so far).  This can be removed once the object
* store is no longer global and instead is a member of the repository
* object.
*/

Since the object store is now a member of the struct repository, I thought it should be possible to tackle this NEEDSWORK as the commentary suggests. I also thought this modification could help me in the process of supporting threads with --recurse-submodules, as it would simplify a bit grep_submodules()’s critical section. So I worked on a patch to make git-grep stop adding submodules to the list of alternates, as you can check here. The patch already works, but some refactoring is needed.

I ran some tests on a “semi-artificial” repository (linux repo with Git as a submodule), but the patched git-grep didn’t showed a significant time drop. I wonder if I need a bigger repository and/or submodule to really see the differences or if, unfortunatelly, this modification ended up not being as effective…

Next steps

My plan for this week is to keep working on a patch on top of the series I sent earlier to allow threading with --recurse-submodules and --textconv. For now, I’ve been focusing on --recurse-submodules, as I think I know more of its code.

One way to proceed towards our goal would be to move the obj_read_mutex down from read_object_file_extended() to oid_object_info_extended(). With this, we could potentially avoid the race conditions between the threads’ calls to read_object_file_extended() and parse_object_or_die() (which calls oid_object_info_extended()).

Nevertheless, I recently discovered that, just by enabling threads when cached (without my patchset), --recurse-submodules already results in race conditions. The problem seems to envolve the userdiff calls as well. I didn’t get to investigate this enought yet, but that should be one of my next tasks.

Finally, I also want to test the “don’t add submodules to alternates” patch, somehow, to see if it is worthy. But I don’t know how yet.