Re: [PATCH 2/4] dir.c::match_basename(): pay attention to the length of string parameters

On Tue, Mar 26, 2013 at 02:55:59PM -0400, Jeff King wrote:

quoted

 * Otherwise, make sure we use only the counted part of the strings
   when calling fnmatch_icase().  Because these counted strings are
   full strings most of the time, avoid unnecessary allocation.

I think this is OK, with the intention that we would eventually drop the
allocations from your third bullet point in favor of using a
byte-counted version of fnmatch (i.e., nwildmatch). But until then we're
going to see a performance drop.

The pattern is usually going to be NUL-terminated at the length counter,
but every time we feed a directory, it's going to run into this
allocation. And we do it once for _every_ directory against _every_
wildcard gitignore pattern. So I think it is probably going to be
measurable. I guess we can try measuring it on something like WebKit,
which has plenty of both directories and gitattributes.

I timed this doing "git archive HEAD" on webkit.git before and after. It
actually ended up not mattering much (I think because it is only the
directories which are affected, not each individually path, so it's a
much smaller number than you'd think). The best-of-five timing was
slightly slower, but was within the noise.

So I do still think it would make sense to go to a byte-limited version
of fnmatch eventually, just for code cleanliness and predictability of
performance, but this is really not a bad solution in the interim.

-Peff