Re: [PATCH v1 1/4] fastindex: speed up index load through parallelization

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 15, 2017, 04:40 UTC
Message-ID: <xmqq1sl017dw.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <9ba23d2c-2198-55d7-5a02-69879fbbb3cb@gmail.com>

Ben Peart <peartben@gmail.com> writes:

Show 6 quoted lines

> OK.  I'll call this new extension "EOIE" ("end of index
> entries"). Other than the standard header/footer, it will only contain
> a 32 bit offset to the beginning of the extension entries.  I'll
> always write out that extension unless you would prefer it be behind a
> setting (if so, what do you want it called and what should the default
> be)?  I won't add support in update-index for this extension.

To make it robust, if I were doing this, I would at least add a checksum of some sort.

As each extension section consists of 4-byte extension type, 4-byte size, followed by that many bytes of the "meat" of the section, what I had in mind when I suggested this backpointer was something like:

    "EOIE" <32-bit size> <32-bit offset> <20-byte hash>

where the size of the extension section is obviously 24-byte to cover the offset plus hash, and the hash is computed over extension types and their sizes (but not their contents---this is not about protecting against file corruption and not worth wasting the cycles for hashing) for all the extension sections this index file has (except for "EOIE" at the end, for obvious reasons). E.g. if we have "TREE" extension that is N-bytes long, "REUC" extension that is M-bytes long, followed by "EOIE", then the hash would be

	SHA-1("TREE" + <binary representation of N> +
	      "REUC" + <binary representation of M>)

Then the reader would

 - Seek back 32-byte from the trailer to ensure it sees "EOIE"
   followed by a correct size (24?)

 - Jump to the offset and find 4-bytes that presumably is the type
   of the first extension, followed by its size.

 - Feed these 8-bytes to the hash, skip that section based on its
   size (while making sure we won't run off the end of the file,
   which is a sign that we thought EOIE exists when there wasn't).
   Repeat this until we hit where we found "EOIE" (or we notice our
   mistake by overrunning it).

 - Check the hash to make sure we got it right.

Show 13 quoted lines

> Since the goal was to find a way to load the IEOT extension before the
> cache entries, I'll also refactor the extension reading loop into a
> function that takes a function pointer and add a
> preread_index_extension() function that can be passed in when that
> loop is run before the cache entries are loaded.  When the loop is run
> again after the cache entries are loaded, it will pass in the existing
> read_index_extension() function.  Extensions can then choose which
> function they want to be loaded in.
>
> The code to read/write/use the IEOT to parallelize the cache entry
> loading will stay behind a config setting that defaults to false (at
> least for now).  I'll stick with "core.fastindex" until someone can
> (please) propose a better name.

Sounds good.