Re: [PATCH v4 3/3] regex: use regexec_buf()

From: Jeff King <peff@peff.net>
Date: Sep 21, 2016, 22:03 UTC
Message-ID: <20160921220310.5wk76qdsjyhvstk4@sigill.intra.peff.net>
In-Reply-To: <53f3609d99c865d59d7bfd8219a5334339e9e6bc.1474482164.git.johannes.schindelin@gmx.de>

On Wed, Sep 21, 2016 at 08:24:14PM +0200, Johannes Schindelin wrote:

Show 11 quoted lines

> The new regexec_buf() function operates on buffers with an explicitly
> specified length, rather than NUL-terminated strings.
> 
> We need to use this function whenever the buffer we want to pass to
> regexec() may have been mmap()ed (and is hence not NUL-terminated).
> 
> Note: the original motivation for this patch was to fix a bug where
> `git diff -G <regex>` would crash. This patch converts more callers,
> though, some of which explicitly allocated and constructed
> NUL-terminated strings (or worse: modified read-only buffers to insert
> NULs).

Nice. I probably would have split these into their own patch, but I think it is OK here.

Show 6 quoted lines

> @@ -228,18 +227,16 @@ static long ff_regexp(const char *line, long len,
>  			len--;
>  	}
>  
> -	line_buffer = xstrndup(line, len); /* make NUL terminated */
> -

Nice to see this one going away in particular, since it's called quite a lot. According to perf, "git log -p" on git.git drops about 1.5 million malloc calls (about 9% of the total). And here are best-of-five results for that same command:

  [before]
  real    0m14.676s
  user    0m13.988s
  sys     0m0.676s

  [after]
  real    0m14.394s
  user    0m13.624s
  sys     0m0.760s

Not a _huge_ improvement, but more significant than the run-to-run noise.

-Peff