From: Ben Peart <peartben@gmail.com>
Date: Mon, 30 Oct 2017 12:48:48 GMT
Subject: Re: [PATCH v2] read_index_from(): Skip verification of the cache entry order to speed index loading
Message-ID: <11666ccf-6406-d585-f519-7a1934c2973a@gmail.com>
In-Reply-To: <20171024144544.7544-1-benpeart@microsoft.com>

Any updates or thoughts on this one?  While the patch has become quite 
trivial, it does results in a savings of 5%-15% in index load time.

I thought the compromise of having this test only run when DEBUG is 
defined should limit it to developer builds (hopefully everyone 
developing on git is running DEBUG builds :)).  Since the test is trying 
to detect buggy code when writing the index, I thought that was the 
right time to test/catch any issues.

I am working on other, more substantial savings for index load times 
(stay tuned) but this seemed like a small simple way to help speed 
things up.

On 10/24/2017 10:45 AM, Ben Peart wrote:
> There is code in post_read_index_from() to detect out of order cache
> entries when reading an index file.  This order verification adds cost
> to read_index_from() that grows with the size of the index.
> 
> Put this on-disk data-structure validation code behind an #ifdef DEBUG
> so only debug builds have to pay the cost.
> 
> The effect can be seen using t/perf/p0002-read-cache.sh:
> 
> Test w/git repo                       HEAD            HEAD~1
> ----------------------------------------------------------------------------
> read_cache/discard_cache 1000 times   0.42(0.01+0.09) 0.48(0.01+0.09) +14.3%
> read_cache/discard_cache 1000 times   0.41(0.03+0.04) 0.49(0.00+0.10) +19.5%
> read_cache/discard_cache 1000 times   0.42(0.03+0.06) 0.49(0.06+0.04) +16.7%
> 
> Test w/10K files                      HEAD            HEAD~1
> ---------------------------------------------------------------------------
> read_cache/discard_cache 1000 times   1.58(0.04+0.00) 1.71(0.00+0.07) +8.2%
> read_cache/discard_cache 1000 times   1.64(0.01+0.07) 1.76(0.01+0.09) +7.3%
> read_cache/discard_cache 1000 times   1.62(0.03+0.04) 1.71(0.00+0.04) +5.6%
> 
> Test w/100K files                     HEAD             HEAD~1
> -----------------------------------------------------------------------------
> read_cache/discard_cache 1000 times   25.85(0.00+0.06) 27.35(0.01+0.06) +5.8%
> read_cache/discard_cache 1000 times   25.82(0.01+0.07) 27.25(0.01+0.07) +5.5%
> read_cache/discard_cache 1000 times   26.00(0.01+0.07) 27.36(0.06+0.03) +5.2%
> 
> Test with 1,000K files                HEAD              HEAD~1
> -------------------------------------------------------------------------------
> read_cache/discard_cache 1000 times   200.61(0.01+0.07) 218.23(0.03+0.06) +8.8%
> read_cache/discard_cache 1000 times   201.62(0.03+0.06) 217.86(0.03+0.06) +8.1%
> read_cache/discard_cache 1000 times   201.64(0.01+0.09) 217.89(0.03+0.07) +8.1%
> 
> Signed-off-by: Ben Peart <benpeart@microsoft.com>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> 
> Notes:
>      Base Ref: master
>      Web-Diff: https://github.com/benpeart/git/commit/95e20f17ff
>      Checkout: git fetch https://github.com/benpeart/git no_ce_order-v2 && git checkout 95e20f17ff
>      
>      ### Interdiff (v1..v2):
>      
>      ### Patches
> 
>   read-cache.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/read-cache.c b/read-cache.c
> index 65f4fe8375..fc90ec0fce 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -1664,6 +1664,7 @@ static struct cache_entry *create_from_disk(struct ondisk_cache_entry *ondisk,
>   	return ce;
>   }
>   
> +#ifdef DEBUG
>   static void check_ce_order(struct index_state *istate)
>   {
>   	unsigned int i;
> @@ -1685,6 +1686,7 @@ static void check_ce_order(struct index_state *istate)
>   		}
>   	}
>   }
> +#endif
>   
>   static void tweak_untracked_cache(struct index_state *istate)
>   {
> @@ -1720,7 +1722,9 @@ static void tweak_split_index(struct index_state *istate)
>   
>   static void post_read_index_from(struct index_state *istate)
>   {
> +#ifdef DEBUG
>   	check_ce_order(istate);
> +#endif
>   	tweak_untracked_cache(istate);
>   	tweak_split_index(istate);
>   }
> 
> base-commit: c52ca88430e6ec7c834af38720295070d8a1e330
>