bug#71094: [PATCH] Prefer to run find and grep in parallel in rgrep

[Spencer Baugh <sbaugh@janestreet.com>]

[-- Attachment #1: Type: text/plain, Size: 2398 bytes --]

Eli Zaretskii <eliz@gnu.org> writes:

>> Cc: Glenn Morris <rgm@gnu.org>, dmitry@gutov.dev
>> From: Spencer Baugh <sbaugh@janestreet.com>
>> Date: Tue, 21 May 2024 10:35:07 -0400
>> 
>> grep.el prefers to run "find" and "xargs grep" in a pipeline,
>> which means that "find" can continue searching the filesystem
>> while "xargs grep" searches files.  If find and xargs don't
>> support the flags required for this behavior, grep.el will fall
>> back to using the -exec flags to "find", which meant "find" will
>> wait for each "grep" process to complete before continuing to
>> search the filesystem tree.  This behavior is controlled by
>> grep-find-use-xargs; `gnu' produces the pipeline and `exec' is
>> the slower fallback.
>> 
>> In f3ca7378c1336b3ff98ecb5a99a98c7b2eceece9, the `exec-plus'
>> option was added for grep-find-use-xargs, which improves on
>> `exec' by running one "grep" process to search multiple files,
>> which `gnu' (by using xargs) already did.  However, the change
>> erroneously added the `exec-plus' case before the `gnu' case in
>> the autodetection code in grep-compute-defaults, so `exec-plus'
>> would be used even if `gnu' was supported.
>> 
>> This change just swaps the two cases, so the faster `gnu' option
>> is once again used in preference to `exec-plus'.  In my
>> benchmarking on a large repository, this provides a ~40%
>> speedup.
>
> With how many files did you measure the 40% speedup?

700k

> Can you show the performance with much fewer and much more files than
> what you used?

Much more is maybe hard, but much fewer is easy: with 212 files (a
subset of the original directory I searched), there's no performance
change.

> I suspect that the effect depends on that.  (It also depends on the
>system limit on the number of files and the length of the command line
>that xargs can use.)  The argument about 'find' waiting is no longer
>relevant with 'exec-plus', since in most cases there will be just one
>invocation of 'grep'.

True, it only matters when the directory tree contains more files than
can be passed to a single invocation of grep.

> In any case, please modify the patch so that 'exec-plus' is still
> preferred on MS-Windows (because most Windows ports of xargs are IME
> abysmally buggy, so better avoided as much as possible).
>
> A comment there with the justification of the order will also be
> appreciated.

Done, attached.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Prefer-to-run-find-and-grep-in-parallel-in-rgrep.patch --]
[-- Type: text/x-patch, Size: 3043 bytes --]

From e7fbfe431ae1f4f004f1d92db2f3b011b30ff682 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh@janestreet.com>
Date: Tue, 21 May 2024 10:32:45 -0400
Subject: [PATCH] Prefer to run find and grep in parallel in rgrep

grep.el prefers to run "find" and "xargs grep" in a pipeline,
which means that "find" can continue searching the filesystem
while "xargs grep" searches files.  If find and xargs don't
support the flags required for this behavior, grep.el will fall
back to using the -exec flags to "find", which meant "find" will
wait for each "grep" process to complete before continuing to
search the filesystem tree.  This behavior is controlled by
grep-find-use-xargs; `gnu' produces the pipeline and `exec' is
the slower fallback.

In f3ca7378c1336b3ff98ecb5a99a98c7b2eceece9, the `exec-plus'
option was added for grep-find-use-xargs, which improves on
`exec' by running one "grep" process to search multiple files,
which `gnu' (by using xargs) already did.  However, the change
erroneously added the `exec-plus' case before the `gnu' case in
the autodetection code in grep-compute-defaults, so `exec-plus'
would be used even if `gnu' was supported.

This change just swaps the two cases, so the faster `gnu' option
is once again used in preference to `exec-plus'.  In my
benchmarking on a large repository, this provides a ~40%
speedup.

Also, we completely avoid running xargs on MS-Windows, because Eli
Zaretskii <eliz@gnu.org> writes:

> most Windows ports of xargs are IME abysmally buggy, so better avoided
> as much as possible

* lisp/progmodes/grep.el (grep-compute-defaults): Prefer `gnu' for
grep-find-use-xargs over `exec-plus', but not on Windows.  (bug#71094)
---
 lisp/progmodes/grep.el | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/lisp/progmodes/grep.el b/lisp/progmodes/grep.el
index 0a9de04fce1..ce54c57aabc 100644
--- a/lisp/progmodes/grep.el
+++ b/lisp/progmodes/grep.el
@@ -812,15 +812,23 @@ grep-compute-defaults
 	(unless grep-find-use-xargs
 	  (setq grep-find-use-xargs
 		(cond
-		 ((grep-probe find-program
-			      `(nil nil nil ,(null-device) "-exec" "echo"
-				    "{}" "+"))
-		  'exec-plus)
+                 ;; For performance, we want:
+                 ;; A. Run grep on batches of files (instead of one grep per file)
+                 ;; B. If the directory is large and we need multiple batches,
+                 ;;    run find in parallel with a running grep.
+                 ;; "find | xargs grep" gives both A and B
 		 ((and
+                   (not (eq system-type 'windows-nt))
 		   (grep-probe
                     find-program `(nil nil nil ,(null-device) "-print0"))
 		   (grep-probe xargs-program '(nil nil nil "-0" "echo")))
 		  'gnu)
+                 ;; "find -exec {} +" gives A but not B
+		 ((grep-probe find-program
+			      `(nil nil nil ,(null-device) "-exec" "echo"
+				    "{}" "+"))
+		  'exec-plus)
+                 ;; "find -exec {} ;" gives neither A nor B.
 		 (t
 		  'exec))))
 	(unless grep-find-command
-- 
2.39.3