Re: [PATCH] Optimize usage of grep by passing -q

From: Jeff King <peff@peff.net>
Date: Nov 17, 2015, 22:37 UTC
Message-ID: <20151117223711.GC27862@sigill.intra.peff.net>
In-Reply-To: <CAGZ79kYCNdDV2cDZSiugx6EuY_0BEgGMJJVm8EBWqPEE77sqbw@mail.gmail.com>

On Mon, Nov 16, 2015 at 05:04:24PM -0800, Stefan Beller wrote:

Show 17 quoted lines

> >> This is true for the gnu version of grep. I am not sure if all
> >> versions of grep support this optimization. In case it is not,
> >> we'd revert this patch.
> >
> > POSIX specifies -q, so you should be fine.
> > http://pubs.opengroup.org/onlinepubs/9699919799/utilities/grep.html
> >
> 
> From http://www.gnu.org/software/grep/manual/grep.html :
> [...]
> Portability note: unlike GNU grep, 7th Edition Unix grep did not
> conform to POSIX, because it lacked -q and its -s option behaved like
> GNU grep's -q option.1
> USG-style grep also lacked -q but its -s option behaved like GNU
> grep's. Portable shell scripts should avoid both -q and -s and should
> redirect standard and error output to /dev/null instead. (-s is
> specified by POSIX.)

I wonder what the current state of "most" systems is. 7th Edition Unix is probably old enough for us not to worry about. :)

For the git project, being in POSIX is not an automatic pass for a feature. We care about real systems. I note that we do have quite a bit of "grep -q" in the test scripts, but not in the actual git-scripts.

This came up as recently as 2008 (e.g., aadbe44), but I don't recall anybody complaining recently. Perhaps Solaris grep finally grew a "-q" option. Or maybe nobody runs the tests there anymore.

Since this is an optimization, I'd be more interested if we had numbers for the improvement. Are these files really big enough that grepping the rest of the file is noticeable versus the cost of starting grep in the first place?

If this is something measurable, we might be able to make it a build flag (e.g., by wrapping these grep invocations in a shell function in git-sh-setup.sh, and picking the implementation at build time).

-Peff