enable fscache while iterating every local refs

[atetubou]

When I do git fetch, git reads entries in .git/objects/pack for every refs in local repository.

By enabling fscache, directory list up in .git/objects/pack for each refs is cached.
Without fscache, such behavior causes long running time when we do git fetch in local repository having many refs.

This patch improves execution time in such case, especially in very large repository like chromium.
In my windows workstation, this patch improves git fetch time from more than 3 minutes to less than 20 seconds for chromium repository.

Signed-off-by: Takuto Ikuta [email protected]

[PhilipOakley]

Doesn't this (enable_fscache) need switching off at some point?

The preload-index.c there are complementary settings as part of the PTHREADS code. However the other two uses don't appear to have the un-setting part. Dunno.

Which ever way it is, could you clarify the result in the commit message? (force push you branch to create the updated PR in this thread..)

[atetubou]

Thank you for comment. I updated commit to disable fscache after iteration.

[PhilipOakley]

It took me a bit of searching..

The enable_fscache() is redefined either to a noop /* */ or fscache_enable() - its is difficult to see the swap around in the function names !

The fscache_enable() is then defined in \git\compat\win32\fscache.c, so you should be able to check there if it is "The Right Thing" (TM) to unset the setting, hence my original question. The commit message should reassure the reader that it is the right thing to do.

It wasn't clear to me why we do not need to unset it in the other two use cases. The message should help clarify (maybe it depends on whether we have a long running command/ command in multiple threads)

[atetubou]

The fscache_enable() is then defined in \git\compat\win32\fscache.c, so you should be able to check there if it is "The Right Thing" (TM) to unset the setting, hence my original question. The commit message should reassure the reader that it is the right thing to do.

Updated bit again, do I understand what you said?

[dscho]

It is always a pleasure to come back from vacation and see new contributions, in particular when two contributors worked together to further improve them. Thank you so much @atetubou and @PhilipOakley!

[PhilipOakley]

@dscho thanks for the support. The main praise must go to @atetubou for the work. I was mainly reflecting on my ignorance of fscache_enable (and enable_fscache ;) , hence my requests to see if the code needed the 'off' swich step. Plus I thought it was already switched on in GfW by default. Any thoughs @dscho?

[dscho]

I thought it was already switched on in GfW by default.

I did that in some topic branch for testing, it needs a patch: dscho@f3cac14

Stupidly, I forgot to record details of the "why?", so I'll have to reconstruct them before I can cherry-pick that to master.

Also, there were some bad interactions between sparse checkouts and fscache, that was the reason I did not switch it to "on" by default. I'll have to check back with @jeffhostetler who performed those tests whether the issues have been resolved.