GNU bug report logs - #69775
[PATCH] Use regexp-opt in dired-omit-regexp

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 69775 in the body.
You can then email your comments to 69775 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

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

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Use regexp-opt in dired-omit-regexp
Date: Wed, 13 Mar 2024 11:01:05 -0400

[Message part 1 (text/plain, inline)]

Tags: patch


In my benchmarking, for large dired buffers, using regexp-opt provides
around a 3x speedup in omitting.

regexp-opt takes around 5 milliseconds, so to avoid slowing down
omitting in small dired buffers we cache the return value.

Since omitting is now 3x faster, increase dired-omit-size-limit by 3x.

* lisp/dired-x.el (dired-omit--extension-regexp-cache): Add.
(dired-omit-regexp): Use regexp-opt.
(dired-omit-size-limit): Increase, since omitting is now faster.

In GNU Emacs 29.2.50 (build 4, x86_64-pc-linux-gnu, X toolkit, cairo
 version 1.15.12, Xaw scroll bars) of 2024-02-28 built on
 igm-qws-u22796a
Repository revision: 46e23709d37943a20faa735c97af520196a443e9
Repository branch: emacs-29
Windowing system distributor 'The X.Org Foundation', version 11.0.12011000
System Description: Rocky Linux 8.9 (Green Obsidian)

Configured using:
 'configure 'CFLAGS=-O0 -g3' --with-gif=ifavailable
 --with-x-toolkit=lucid'

[0001-Use-regexp-opt-in-dired-omit-regexp.patch (text/patch, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: 69775 <at> debbugs.gnu.org
Subject: Re: bug#69775: [PATCH] Use regexp-opt in dired-omit-regexp
Date: Thu, 14 Mar 2024 13:00:00 +0200

> From: Spencer Baugh <sbaugh <at> janestreet.com>
> Date: Wed, 13 Mar 2024 11:01:05 -0400
> 
> In my benchmarking, for large dired buffers, using regexp-opt provides
> around a 3x speedup in omitting.

Can you show a recipe for such benchmarking?  I'd like to try that on
my systems.

Also, what is the slowdown in the (improbable, but possible) case
where dired-omit-extensions change for each call of dired-omit-regexp?

> regexp-opt takes around 5 milliseconds, so to avoid slowing down
> omitting in small dired buffers we cache the return value.
> 
> Since omitting is now 3x faster, increase dired-omit-size-limit by 3x.
> 
> * lisp/dired-x.el (dired-omit--extension-regexp-cache): Add.
> (dired-omit-regexp): Use regexp-opt.
> (dired-omit-size-limit): Increase, since omitting is now faster.

I'm okay with these changes, but:

  . the change in the default value of dired-omit-size-limit should be
    called out in NEWS
  . please document this variable in the dired-x.texi manual, where we
    document all the other variables relevant to dired-omit mode.
  . the doc string of dired-omit-size-limit is embarrassingly
    unhelpful, so bonus points for fixing that as well

Thanks.

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

From: sbaugh <at> catern.com
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Spencer Baugh <sbaugh <at> janestreet.com>, 69775 <at> debbugs.gnu.org
Subject: Re: bug#69775: [PATCH] Use regexp-opt in dired-omit-regexp
Date: Sat, 16 Mar 2024 17:15:52 +0000 (UTC)

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Spencer Baugh <sbaugh <at> janestreet.com>
>> Date: Wed, 13 Mar 2024 11:01:05 -0400
>> 
>> In my benchmarking, for large dired buffers, using regexp-opt provides
>> around a 3x speedup in omitting.
>
> Can you show a recipe for such benchmarking?  I'd like to try that on
> my systems.
>
> Also, what is the slowdown in the (improbable, but possible) case
> where dired-omit-extensions change for each call of dired-omit-regexp?

Yes, run the following after applying the patch:

(require 'dired)
(require 'dired-x)
(require 'cl-lib)

(defun dired-omit-regexp-old ()
  (concat (if dired-omit-files (concat "\\(" dired-omit-files "\\)") "")
          (if (and dired-omit-files dired-omit-extensions) "\\|" "")
          (if dired-omit-extensions
              (concat ".";; a non-extension part should exist
                      "\\("
                      (mapconcat 'regexp-quote dired-omit-extensions "\\|")
                      "\\)$")
            "")))

(defun my-do-omit (mode)
  (let ((regexp
	 (cl-case mode
	   (new (dired-omit-regexp))
	   (old (dired-omit-regexp-old))
	   (new-uncached (let ((dired-omit--extension-regexp-cache nil)) (dired-omit-regexp)))
	   (t (error "Bad mode %s" mode)))))
    (dired-mark-if
     (let ((fn (dired-get-filename nil t)))
       (and fn (string-match-p regexp fn)))
     nil)))

(defun my-bench-omit (nfiles ntimes)
  (let ((default-directory (expand-file-name "test-dired-list")))
    (make-directory default-directory t)
    (dolist (file (directory-files "." t "test-file"))
      (delete-file file))
    (dotimes (i nfiles)
      (write-region "" nil (format "test-file%s" i) nil 'nomessage nil 'excl))
    (let ((dired-omit-mode nil))
      (with-current-buffer (let ((inhibit-message t)) (dired-noselect default-directory))
	(revert-buffer)
	(message "files %s, ntimes %s: new %s old %s new-uncached %s"
		 nfiles ntimes
		 (car (benchmark-call (lambda () (my-do-omit 'new)) ntimes))
		 (car (benchmark-call (lambda () (my-do-omit 'old)) ntimes))
		 (car (benchmark-call (lambda () (my-do-omit 'new-uncached)) ntimes)))
	))))

(my-bench-omit 1 100)
(my-bench-omit 10 100)
(my-bench-omit 100 100)
(my-bench-omit 1000 100)
(my-bench-omit 10000 100)

For me, I get:
$ ./src/emacs -Q --batch -l ../emacs-29/bench-omit.elc
files 1, ntimes 100: new 0.008839979999999999 old 0.018162129 new-uncached 0.031399762
files 10, ntimes 100: new 0.012037615 old 0.040232355000000004 new-uncached 0.037990543
files 100, ntimes 100: new 0.07368538100000001 old 0.314905271 new-uncached 0.10006527300000001
files 1000, ntimes 100: new 0.669103498 old 3.076339984 new-uncached 0.693134644
files 10000, ntimes 100: new 6.336211434 old 30.926320486 new-uncached 6.442762152999999

So the performance improvement is quite substantial for large
directories.

new-uncached is the performance if dired-omit-extensions changes on each
call of dired-omit-regexp.  For a directory of 1 file, the overhead of
recomputing regexp-opt every time makes the performance perhaps 2x-3x
worse, but around 10 files the performance improvement from regexp-opt
exceeds the overhead, and above that the uncached version still
outperforms the old version substantially.

If dired-omit-extensions doesn't change every time, the performance is
improved even for directories of 1 file.

>> regexp-opt takes around 5 milliseconds, so to avoid slowing down
>> omitting in small dired buffers we cache the return value.
>> 
>> Since omitting is now 3x faster, increase dired-omit-size-limit by 3x.
>> 
>> * lisp/dired-x.el (dired-omit--extension-regexp-cache): Add.
>> (dired-omit-regexp): Use regexp-opt.
>> (dired-omit-size-limit): Increase, since omitting is now faster.
>
> I'm okay with these changes, but:
>
>   . the change in the default value of dired-omit-size-limit should be
>     called out in NEWS
>   . please document this variable in the dired-x.texi manual, where we
>     document all the other variables relevant to dired-omit mode.
>   . the doc string of dired-omit-size-limit is embarrassingly
>     unhelpful, so bonus points for fixing that as well
>
> Thanks.

Certainly, updated patch attached.

[0001-Use-regexp-opt-in-dired-omit-regexp.patch (text/x-patch, inline)]

From ca0ff9d40b85b281e25b998528c1e1a71b9e70c5 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh <at> catern.com>
Date: Sat, 16 Mar 2024 17:11:24 +0000
Subject: [PATCH] Use regexp-opt in dired-omit-regexp

In my benchmarking, for large dired buffers, using regexp-opt provides
around a 3x speedup in omitting.

regexp-opt takes around 5 milliseconds, so to avoid slowing down
omitting in small dired buffers we cache the return value.

Since omitting is now 3x faster, increase dired-omit-size-limit by 3x.
Also, document dired-omit-size-limit better.

* doc/misc/dired-x.texi (Omitting Variables): Document
dired-omit-size-limit.
* etc/NEWS: Announce increase of dired-omit-size-limit.
* lisp/dired-x.el (dired-omit--extension-regexp-cache): Add.
(dired-omit-regexp): Use regexp-opt. (bug#69775)
(dired-omit-size-limit): Increase and improve docs.
---
 doc/misc/dired-x.texi |  8 ++++++++
 etc/NEWS              |  6 ++++++
 lisp/dired-x.el       | 26 ++++++++++++++++++++------
 3 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/doc/misc/dired-x.texi b/doc/misc/dired-x.texi
index 4cad016a0f6..66045c5f759 100644
--- a/doc/misc/dired-x.texi
+++ b/doc/misc/dired-x.texi
@@ -346,6 +346,14 @@ Omitting Variables
 match the file name relative to the buffer's top-level directory.
 @end defvar
 
+@defvar dired-omit-size-limit
+If non-@code{nil}, omitting will be skipped if the directory listing
+exceeds this size in bytes.  Since omitting can be slow for very large
+directories, this avoids having to wait before seeing the directory.
+This variable is ignored when @code{dired-omit-mode} is called
+interactively, such as by @code{C-x M-o}, so you can still enable
+omitting in the directory after the initial display.
+
 @cindex omitting additional files
 @defvar dired-omit-marker-char
 Temporary marker used by Dired to implement omitting.  Should never be used
diff --git a/etc/NEWS b/etc/NEWS
index b4a1c887f2e..fdface7aa0c 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -668,6 +668,12 @@ marked or clicked on files according to the OS conventions.  For
 example, on systems supporting XDG, this runs 'xdg-open' on the
 files.
 
+*** The default value of 'dired-omit-size-limit' has increased.
+After performance improvements to omitting in large directories, the new
+default value is 300k, up from 100k.  This means 'dired-omit-mode' will
+omit files in directories whose directory listing is up to 300 kilobytes
+in size.
+
 +++
 *** 'dired-listing-switches' handles connection-local values if exist.
 This allows to customize different switches for different remote machines.
diff --git a/lisp/dired-x.el b/lisp/dired-x.el
index 62fdd916e69..d7d15028489 100644
--- a/lisp/dired-x.el
+++ b/lisp/dired-x.el
@@ -77,12 +77,17 @@ dired-vm-read-only-folders
 		 (other :tag "non-writable only" if-file-read-only))
   :group 'dired-x)
 
-(defcustom dired-omit-size-limit 100000
-  "Maximum size for the \"omitting\" feature.
+(defcustom dired-omit-size-limit 300000
+  "Maximum buffer size for `dired-omit-mode'.
+
+Omitting will be skipped if the directory listing exceeds this size in
+bytes.  This variable is ignored when `dired-omit-mode' is called
+interactively.
+
 If nil, there is no maximum size."
   :type '(choice (const :tag "no maximum" nil) integer)
   :group 'dired-x
-  :version "29.1")
+  :version "30.1")
 
 (defcustom dired-omit-case-fold 'filesystem
   "Determine whether \"omitting\" patterns are case-sensitive.
@@ -506,14 +511,23 @@ dired-omit-expunge
                                       (re-search-forward dired-re-mark nil t))))
         count)))
 
+(defvar dired-omit--extension-regexp-cache
+  nil
+  "A cache of `regexp-opt' applied to `dired-omit-extensions'.
+
+This is a cons whose car is a list of strings and whose cdr is a
+regexp produced by `regexp-opt'.")
+
 (defun dired-omit-regexp ()
+  (unless (equal dired-omit-extensions (car dired-omit--extension-regexp-cache))
+    (setq dired-omit--extension-regexp-cache
+          (cons dired-omit-extensions (regexp-opt dired-omit-extensions))))
   (concat (if dired-omit-files (concat "\\(" dired-omit-files "\\)") "")
           (if (and dired-omit-files dired-omit-extensions) "\\|" "")
           (if dired-omit-extensions
               (concat ".";; a non-extension part should exist
-                      "\\("
-                      (mapconcat 'regexp-quote dired-omit-extensions "\\|")
-                      "\\)$")
+                      (cdr dired-omit--extension-regexp-cache)
+                      "$")
             "")))
 
 ;; Returns t if any work was done, nil otherwise.
-- 
2.44.0

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: sbaugh <at> catern.com
Cc: sbaugh <at> janestreet.com, 69775 <at> debbugs.gnu.org
Subject: Re: bug#69775: [PATCH] Use regexp-opt in dired-omit-regexp
Date: Thu, 21 Mar 2024 12:38:28 +0200

> From: sbaugh <at> catern.com
> Date: Sat, 16 Mar 2024 17:15:52 +0000 (UTC)
> Cc: Spencer Baugh <sbaugh <at> janestreet.com>, 69775 <at> debbugs.gnu.org
> 
> $ ./src/emacs -Q --batch -l ../emacs-29/bench-omit.elc
> files 1, ntimes 100: new 0.008839979999999999 old 0.018162129 new-uncached 0.031399762
> files 10, ntimes 100: new 0.012037615 old 0.040232355000000004 new-uncached 0.037990543
> files 100, ntimes 100: new 0.07368538100000001 old 0.314905271 new-uncached 0.10006527300000001
> files 1000, ntimes 100: new 0.669103498 old 3.076339984 new-uncached 0.693134644
> files 10000, ntimes 100: new 6.336211434 old 30.926320486 new-uncached 6.442762152999999
> 
> So the performance improvement is quite substantial for large
> directories.
> 
> new-uncached is the performance if dired-omit-extensions changes on each
> call of dired-omit-regexp.  For a directory of 1 file, the overhead of
> recomputing regexp-opt every time makes the performance perhaps 2x-3x
> worse, but around 10 files the performance improvement from regexp-opt
> exceeds the overhead, and above that the uncached version still
> outperforms the old version substantially.

SGTM, thanks.

> Certainly, updated patch attached.
> 
> +@defvar dired-omit-size-limit
> +If non-@code{nil}, omitting will be skipped if the directory listing
> +exceeds this size in bytes.

I'd rephrase

   If non-@code{nil}, @code{dired-omit-mode} will be effectively
   disabled in directories whose listing has size (in bytes) larger
   than the value of this option.

> +*** The default value of 'dired-omit-size-limit' has increased.
                                                    ^^^
"was" is better there.

> +(defcustom dired-omit-size-limit 300000
> +  "Maximum buffer size for `dired-omit-mode'.
> +
> +Omitting will be skipped if the directory listing exceeds this size in
                    ^^^^^^^
"disabled"

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

From: sbaugh <at> catern.com
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: sbaugh <at> janestreet.com, 69775 <at> debbugs.gnu.org
Subject: Re: bug#69775: [PATCH] Use regexp-opt in dired-omit-regexp
Date: Sat, 23 Mar 2024 13:29:06 +0000 (UTC)

[Message part 1 (text/plain, inline)]

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: sbaugh <at> catern.com
>> Date: Sat, 16 Mar 2024 17:15:52 +0000 (UTC)
>> Cc: Spencer Baugh <sbaugh <at> janestreet.com>, 69775 <at> debbugs.gnu.org
>> 
>> $ ./src/emacs -Q --batch -l ../emacs-29/bench-omit.elc
>> files 1, ntimes 100: new 0.008839979999999999 old 0.018162129 new-uncached 0.031399762
>> files 10, ntimes 100: new 0.012037615 old 0.040232355000000004 new-uncached 0.037990543
>> files 100, ntimes 100: new 0.07368538100000001 old 0.314905271 new-uncached 0.10006527300000001
>> files 1000, ntimes 100: new 0.669103498 old 3.076339984 new-uncached 0.693134644
>> files 10000, ntimes 100: new 6.336211434 old 30.926320486 new-uncached 6.442762152999999
>> 
>> So the performance improvement is quite substantial for large
>> directories.
>> 
>> new-uncached is the performance if dired-omit-extensions changes on each
>> call of dired-omit-regexp.  For a directory of 1 file, the overhead of
>> recomputing regexp-opt every time makes the performance perhaps 2x-3x
>> worse, but around 10 files the performance improvement from regexp-opt
>> exceeds the overhead, and above that the uncached version still
>> outperforms the old version substantially.
>
> SGTM, thanks.
>
>> Certainly, updated patch attached.
>> 
>> +@defvar dired-omit-size-limit
>> +If non-@code{nil}, omitting will be skipped if the directory listing
>> +exceeds this size in bytes.
>
> I'd rephrase
>
>    If non-@code{nil}, @code{dired-omit-mode} will be effectively
>    disabled in directories whose listing has size (in bytes) larger
>    than the value of this option.
>
>> +*** The default value of 'dired-omit-size-limit' has increased.
>                                                     ^^^
> "was" is better there.
>
>> +(defcustom dired-omit-size-limit 300000
>> +  "Maximum buffer size for `dired-omit-mode'.
>> +
>> +Omitting will be skipped if the directory listing exceeds this size in
>                     ^^^^^^^
> "disabled"

Adjusted all these, here's the new patch.

[0001-Use-regexp-opt-in-dired-omit-regexp.patch (text/x-diff, inline)]

From 7ae71f994814f3eb7bad3387608e8c0cbe0d2d68 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh <at> catern.com>
Date: Sat, 16 Mar 2024 17:11:24 +0000
Subject: [PATCH] Use regexp-opt in dired-omit-regexp

In my benchmarking, for large dired buffers, using regexp-opt provides
around a 3x speedup in omitting.

regexp-opt takes around 5 milliseconds, so to avoid slowing down
omitting in small dired buffers we cache the return value.

Since omitting is now 3x faster, increase dired-omit-size-limit by 3x.
Also, document dired-omit-size-limit better.

* doc/misc/dired-x.texi (Omitting Variables): Document
dired-omit-size-limit.
* etc/NEWS: Announce increase of dired-omit-size-limit.
* lisp/dired-x.el (dired-omit--extension-regexp-cache): Add.
(dired-omit-regexp): Use regexp-opt. (bug#69775)
(dired-omit-size-limit): Increase and improve docs.
---
 doc/misc/dired-x.texi |  9 +++++++++
 etc/NEWS              |  6 ++++++
 lisp/dired-x.el       | 26 ++++++++++++++++++++------
 3 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/doc/misc/dired-x.texi b/doc/misc/dired-x.texi
index 4cad016a0f6..726b6653d0d 100644
--- a/doc/misc/dired-x.texi
+++ b/doc/misc/dired-x.texi
@@ -346,6 +346,15 @@ Omitting Variables
 match the file name relative to the buffer's top-level directory.
 @end defvar
 
+@defvar dired-omit-size-limit
+If non-@code{nil}, @code{dired-omit-mode} will be effectively disabled
+in directories whose listing has size (in bytes) larger than the value
+of this option.  Since omitting can be slow for very large directories,
+this avoids having to wait before seeing the directory.  This variable
+is ignored when @code{dired-omit-mode} is called interactively, such as
+by @code{C-x M-o}, so you can still enable omitting in the directory
+after the initial display.
+
 @cindex omitting additional files
 @defvar dired-omit-marker-char
 Temporary marker used by Dired to implement omitting.  Should never be used
diff --git a/etc/NEWS b/etc/NEWS
index 50f0ee4a1aa..9324d9f1dcf 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -692,6 +692,12 @@ marked or clicked on files according to the OS conventions.  For
 example, on systems supporting XDG, this runs 'xdg-open' on the
 files.
 
+*** The default value of 'dired-omit-size-limit' was increased.
+After performance improvements to omitting in large directories, the new
+default value is 300k, up from 100k.  This means 'dired-omit-mode' will
+omit files in directories whose directory listing is up to 300 kilobytes
+in size.
+
 +++
 *** 'dired-listing-switches' handles connection-local values if exist.
 This allows to customize different switches for different remote machines.
diff --git a/lisp/dired-x.el b/lisp/dired-x.el
index 62fdd916e69..753d3054d2f 100644
--- a/lisp/dired-x.el
+++ b/lisp/dired-x.el
@@ -77,12 +77,17 @@ dired-vm-read-only-folders
 		 (other :tag "non-writable only" if-file-read-only))
   :group 'dired-x)
 
-(defcustom dired-omit-size-limit 100000
-  "Maximum size for the \"omitting\" feature.
+(defcustom dired-omit-size-limit 300000
+  "Maximum buffer size for `dired-omit-mode'.
+
+Omitting will be disabled if the directory listing exceeds this size in
+bytes.  This variable is ignored when `dired-omit-mode' is called
+interactively.
+
 If nil, there is no maximum size."
   :type '(choice (const :tag "no maximum" nil) integer)
   :group 'dired-x
-  :version "29.1")
+  :version "30.1")
 
 (defcustom dired-omit-case-fold 'filesystem
   "Determine whether \"omitting\" patterns are case-sensitive.
@@ -506,14 +511,23 @@ dired-omit-expunge
                                       (re-search-forward dired-re-mark nil t))))
         count)))
 
+(defvar dired-omit--extension-regexp-cache
+  nil
+  "A cache of `regexp-opt' applied to `dired-omit-extensions'.
+
+This is a cons whose car is a list of strings and whose cdr is a
+regexp produced by `regexp-opt'.")
+
 (defun dired-omit-regexp ()
+  (unless (equal dired-omit-extensions (car dired-omit--extension-regexp-cache))
+    (setq dired-omit--extension-regexp-cache
+          (cons dired-omit-extensions (regexp-opt dired-omit-extensions))))
   (concat (if dired-omit-files (concat "\\(" dired-omit-files "\\)") "")
           (if (and dired-omit-files dired-omit-extensions) "\\|" "")
           (if dired-omit-extensions
               (concat ".";; a non-extension part should exist
-                      "\\("
-                      (mapconcat 'regexp-quote dired-omit-extensions "\\|")
-                      "\\)$")
+                      (cdr dired-omit--extension-regexp-cache)
+                      "$")
             "")))
 
 ;; Returns t if any work was done, nil otherwise.
-- 
2.41.0

Message #22 received at 69775-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: sbaugh <at> catern.com
Cc: sbaugh <at> janestreet.com, 69775-done <at> debbugs.gnu.org
Subject: Re: bug#69775: [PATCH] Use regexp-opt in dired-omit-regexp
Date: Sat, 23 Mar 2024 19:11:16 +0200

> From: sbaugh <at> catern.com
> Date: Sat, 23 Mar 2024 13:29:06 +0000 (UTC)
> Cc: sbaugh <at> janestreet.com, 69775 <at> debbugs.gnu.org
> 
> Adjusted all these, here's the new patch.

Thanks, installed on master, and closing the bug.

Previous Next

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