Re: Git Test Coverage Report (v2.20.0-rc0)

From: Derrick Stolee <stolee@gmail.com>
To: "Ben Peart" <peartben@gmail.com>,
	"git@vger.kernel.org" <git@vger.kernel.org>,
	"Jeff King" <peff@peff.net>, "Stefan Beller" <sbeller@google.com>,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>,
	"Ben Peart" <benpeart@microsoft.com>
Subject: Re: Git Test Coverage Report (v2.20.0-rc0)
Date: Mon, 19 Nov 2018 16:06:25 -0500	[thread overview]
Message-ID: <d7cbe4d1-36ee-2a3c-7ca5-d2e59a8a750f@gmail.com> (raw)
In-Reply-To: <d39320e2-0614-569b-4e82-7f7541589aea@gmail.com>

On 11/19/2018 2:00 PM, Ben Peart wrote:
>
>
> On 11/19/2018 10:40 AM, Derrick Stolee wrote:
>>
>> There are a lot of lines introduced by the IEOT extension in these 
>> commits:
>>
>>  > Ben Peart      3255089ad: ieot: add Index Entry Offset Table 
>> (IEOT) extension
>>  > Ben Peart      3b1d9e045: eoie: add End of Index Entry (EOIE) 
>> extension
>>  > Ben Peart      77ff1127a: read-cache: load cache entries on worker 
>> threads
>>  > Ben Peart      abb4bb838: read-cache: load cache extensions on a 
>> worker thread
>>  > Ben Peart      c780b9cfe: config: add new index.threads config 
>> setting
>>  > Ben Peart      d1664e73a: add: speed up cmd_add() by utilizing 
>> read_cache_preload()
>>  > Ben Peart      fa655d841: checkout: optimize "git checkout -b 
>> <new_branch>"
>>
>
> These should be hit if you run the test suite with 
> GIT_TEST_INDEX_THREADS=2.  Without that, the indexes for the various 
> tests are too small to trigger multi-threaded index reads/writes.
>
> From t/README:
>
> GIT_TEST_INDEX_THREADS=<n> enables exercising the multi-threaded loading
> of the index for the whole test suite by bypassing the default number of
> cache entries and thread minimums. Setting this to 1 will make the
> index loading single threaded.

I updated my build to add GIT_TEST_INDEX_THREADS=2 and I still see lots 
of uncovered stuff, including that load_cache_entries_threaded() is 
never run.

I added the following diff to my repo and ran the test suite manually 
with GIT_TEST_INDEX_THREADS=2 and it didn't fail:

diff --git a/read-cache.c b/read-cache.c
index 4ca81286c0..36502586a2 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2057,6 +2057,9 @@ static unsigned long 
load_cache_entries_threaded(struct index_state *istate, con
         struct load_cache_entries_thread_data *data;
         unsigned long consumed = 0;

+       fprintf(stderr, "load_cache_entries_threaded\n");
+       exit(1);
+
         /* a little sanity checking */
         if (istate->name_hash_initialized)
                 BUG("the name hash isn't thread safe");

Am I missing something? Is there another variable I should add?

When I look for where the GIT_TEST_INDEX_THREADS variable is checked, I 
see that the calls to git_config_get_index_threads() are followed by a 
check for NO_PTHREADS (setting the number of threads to 1 again). Is it 
possible that my compiler environment is not allowing me to even compile 
with threads?

Thanks,
-Stolee



