From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Sat, 11 Mar 2017 20:21:35 GMT
Subject: Re: [PATCH 4/4] ref-filter: use separate cache for contains_tag_algo
Message-ID: <CACBZZX7M9-4Vn4SWRNNkp4ek-D8GwtLKp3pr11SAr6jrk5fEXA@mail.gmail.com>
In-Reply-To: <CACBZZX5A46-ua9y_HxqrqycQviR0uGnStnaPwH9jukzPVak6ug@mail.gmail.com>

On Sat, Mar 11, 2017 at 9:01 PM, Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> On Thu, Mar 9, 2017 at 2:29 PM, Jeff King <peff@peff.net> wrote:
>> [...]
>> @@ -1874,6 +1886,8 @@ int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int
>>                 broken = 1;
>>         filter->kind = type & FILTER_REFS_KIND_MASK;
>>
>> +       init_contains_cache(&ref_cbdata.contains_cache);
>> +
>>         /*  Simple per-ref filtering */
>> [...]
>>
>> +       clear_contains_cache(&ref_cbdata.contains_cache);
>>
>>         /*  Filters that need revision walking */
>>         if (filter->merge_commit)
>
> Shouldn't both of those be guarded by a "if (filter->with_commit)" test?
>
> That init/clear codepath is rather light, but it seems to me that we
> can avoid it entirely if filter->with_commit isn't defined. I've
> tested this locally and it still passes all tests.

Since that seems to work perfectly and we're doing less work I've
incorporated that into the v4 of my series at
<20170311201858.27555-1-avarab@gmail.com>.