On Wed, Nov 9, 2016 at 1:45 AM, Duy Nguyen <pclouds@xxxxxxxxx> wrote:
> On Fri, Oct 28, 2016 at 1:29 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>> The reason why I am bringing this up in this discussion thread on
>> this patch is because I wonder if we would benefit by a similar
>> "let's not do too involved things and be cheap by erring on the safe
>> and lazy side" strategy in the call to ce_path_match() call made in
>> this function to avoid making calls to the attr subsystem.
>>
>> In other words, would it help the system by either simplifying the
>> processing done or reducing the cycle spent in preload_thread() if
>> we could tell ce_path_match() "A pathspec we are checking may
>> require not just the pattern to match but also attributes given to
>> the path to satisfy the criteria, but for the purpose of preloading,
>> pretend that the attribute satisfies the match criteria" (or
>> "pretend that it does not match"), thereby not having to make any
>> call into the attribute subsystem at all from this codepath?
>>
>> The strategy this round takes to make it unnecessary to punt
>> preloading (i.e. dropping "pathspec: disable preload-index when
>> attribute pathspec magic is in use" patch the old series had) is to
>> make the attribute subsystem thread-safe. But that thread-safety in
>> the initial round is based on a single Big Attribute Lock, so it may
>> turn out that the end result performs better for this codepath if we
>> did not to make any call into the attribute subsystem.
>
> It does sound good and I want to say "yes please do this", but is it
> making pathspec api a bit more complex (to express "assume all
> attr-related criteria match")? I guess we can have an api to simply
> filter out attr-related magic (basically set attr_match_nr back to
> zero) then pass a safe (but more relaxing) pathspec to the threaded
> code. That would not add big maintenance burden.
>
So with the current implementation, we already have the shortcut as:
if (item->attr_match_nr && !match_attrs(name, namelen, item))
i.e. if attr_match_nr is zero, we do not even look at the mutexes and such,
so I am not sure what you intend to say in this email?