On 3/27/2017 6:44 PM, Jeff King wrote:
On Mon, Mar 27, 2017 at 09:09:38PM +0000, g...@jeffhostetler.com wrote:

From: Jeff Hostetler <jeffh...@microsoft.com>

Teach git to skip verification of the index SHA in read_index().

This is a performance optimization.  The index file SHA verification
can be considered an ancient relic from the early days of git and only
useful for detecting disk corruption.  For small repositories, this
SHA calculation is not that significant, but for gigantic repositories
this calculation adds significant time to every command.

I added a global "skip_verify_index" variable to control this and
allow it to be tested.

I did not create a config setting for this because of chicken-n-egg
problems with the loading the config and the index.

Hrm, there shouldn't be any dependency of the config on the index (and
there are a handful of options which impact the index already). Did you
try it and run into problems?

Yeah, I tried adding a new "core.verifyindex" property and the
corresponding global variable.  But read_index() and verify_hdr()
was being called BEFORE the config was loaded.  And it wasn't clear
how best to solve that.

The issue was in "git status" where cmd_status() called
status_init_config() which called gitmodules_config() before
git_config().  but gitmodules_config() called read_index(),
so my settings weren't loaded yet in verify_hdr().

I tried switching the order in status_init_config(), but
that caused errors in t7508 with submodules not being handled
properly.

https://github.com/jeffhostetler/git/commits/upstream/core_verify_index

At this point I decided that it wasn't that important to have
this config setting, since we'll probably default it to be faster
and be done with it.


In general, I'd much rather see us either:

  1. Rip the code out entirely if it is not meant to be configurable,
     and cannot be triggered by the actual git binary.

or

  2. Make it configurable, even if most people wouldn't use it. And then
     have a test to exercise it using a git command (unlike the one-off
     test helper, which isn't run at all).

-Peff


I'm OK with (1) if everyone else is.

Jeff

Reply via email to