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. > I am probably being two step ahead of ourselves by saying the above, > which is just something to keep in mind as a possible solution if > performance in this preload codepath becomes an issue when the > pathspec has attributes match specified. -- Duy