GNU bug report logs - #71179
[PATCH] In rgrep, check matching files before excluding files

Previous Next

Full log

View this message in rfc822 format

From: Eli Zaretskii <eliz <at> gnu.org>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: 71179 <at> debbugs.gnu.org
Subject: bug#71179: [PATCH] In rgrep, check matching files before excluding files
Date: Sat, 25 May 2024 09:36:45 +0300

> From: Spencer Baugh <sbaugh <at> janestreet.com>
> Date: Fri, 24 May 2024 16:14:39 -0400
> 
> In my benchmarking, this takes (rgrep "foo" "*.el" "~/src/emacs/trunk/")
> from ~410ms to ~130ms.

Which is a minor improvement at best, possibly a negligible one.  In
my testing (on MS-Windows), I see a barely-tangible improvement: 0.7%.

> Date: Fri, 24 May 2024 23:45:00 +0300
> From: Dmitry Gutov <dmitry <at> gutov.dev>
> 
> > In my benchmarking, this takes (rgrep "foo" "*.el" "~/src/emacs/trunk/")
> > from ~410ms to ~130ms.
> 
> I can confirm improvement here (though not exactly 3x).
> 
> 1.9s to 1.3s in a Linux checkout, for example. Nice.

Which is still quite minor.

> Moving the files exclude instructions to the <F> placeholder is a slight 
> incompatibility

Right, and for that reason, we cannot install this change as-is.  We
need either a different command or a user option controlling the order
(with a good explanation of the effect of the difference).

> Cc: 71179 <at> debbugs.gnu.org
> From: Spencer Baugh <sbaugh <at> janestreet.com>
> Date: Fri, 24 May 2024 16:54:37 -0400
> 
> > Moving the files exclude instructions to the <F> placeholder is a
> > slight incompatibility, but I wonder if there are any custom
> > grep-find-template values which would be bothered by it (that's the
> > only incompatibility I could think of). Perhaps those that currently
> > don't include <X> at all?
> 
> A grep-find-template that doesn't include <X> will indeed start seeing
> ignores based on grep-find-ignored-files in rgrep.  But, such a user can
> just set grep-find-ignored-files to nil and then they'll stop seeing
> ignores again.

That's not a valid argument for changing the default behavior.
Because I could counter-argue that if you don't care about the order
and want those few hundreds of millisecond at all costs, then _you_
can customize the template to your liking, leaving the default
behavior intact.

> Also, for what it's worth, note that the documentation for
> grep-find-template says this:
> 
>   <X> - find options to restrict or expand the directory list
>   <F> - find options to limit the files matched
> 
> So this change makes the documentation more accurate: <X> previously
> also affected the files matched, but now it only affects the directory
> list, as documented.  <F> continues to limit the files matched, as
> before.

Sorry, such incompatible changes are not acceptable, definitely when
the gain is so small.  Correctness trumps speed.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.