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

Previous Next

Package: emacs;

Reported by: Spencer Baugh <sbaugh <at> janestreet.com>

Date: Fri, 24 May 2024 20:15:02 UTC

Severity: normal

Tags: patch

Fixed in version 30.1

Done: Stefan Kangas <stefankangas <at> gmail.com>

Bug is archived. No further changes may be made.

Full log


Message #23 received at 71179 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: sbaugh <at> janestreet.com, 71179 <at> debbugs.gnu.org
Subject: Re: bug#71179: [PATCH] In rgrep, check matching files before
 excluding files
Date: Sat, 25 May 2024 15:51:13 +0300
> Date: Sat, 25 May 2024 15:26:56 +0300
> Cc: 71179 <at> debbugs.gnu.org
> From: Dmitry Gutov <dmitry <at> gutov.dev>
> 
> On 25/05/2024 09:36, Eli Zaretskii wrote:
> >> 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%.
> 
> That's unfortunate, but I think we prioritize GNU systems when making 
> such decisions.

We do.  I just added one more data point.

> >>> 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.
> 
> A 30% improvement is nothing to sneeze at, especially for a code change 
> as simple as this one.

They are 30%, but they are only 600 milliseconds.

> >> 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).
> 
> A user option might work, but before we add one it would be great to 
> understand who are the users that it is for.

The ones for whom the proposed change will affect the results.

> > Sorry, such incompatible changes are not acceptable, definitely when
> > the gain is so small.  Correctness trumps speed.
> 
> Can you think of a specific problematic usage?

Why is that needed?  Isn't it clear that it can happen?




This bug report was last modified 1 year and 40 days ago.

Previous Next


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