~protesilaos/denote

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
4 3

[PATCH v2] Implement predicate on directory-files-recursively for faster searching

Details
Message ID
<20230414200616.5432-1-graham@mgmarlow.com>
DKIM signature
missing
Download raw message
Patch: +26 -15
From: mgmarlow <graham@mgmarlow.com>

Great suggestions! I'm wondering whether we can completely normalize the
use of directory-files-recursively between the two functions. Is there a
reason for them to be different? In my head, I'd imagine the file search
itself is identical--all that needs to change is the use of seq-remove
depending on whether we want to return files or subdirectories.

I brought in your proposed changes and hoisted the predicate function
so it can be shared among both denote-directory-files and
denote-directory-subdirectories.

---
 denote.el | 41 ++++++++++++++++++++++++++---------------
 1 file changed, 26 insertions(+), 15 deletions(-)

diff --git a/denote.el b/denote.el
index d662f29..488c6b1 100644
--- a/denote.el
+++ b/denote.el
@@ -711,6 +711,28 @@ FILE must be an absolute path."
  (string-prefix-p (denote-directory)
                   (expand-file-name default-directory)))

(defun denote--exclude-directory-regexp-p (file)
  "Return non-nil if FILE matches `denote-excluded-directories-regexp'."
  (and denote-excluded-directories-regexp
       (string-match-p denote-excluded-directories-regexp file)))

(defun denote--directory-all-files-recursively ()
  "Return list of all files in variable `denote-directory'.
Avoids traversing dotfiles (unconditionally) and whatever matches
`denote-excluded-directories-regexp'."
  (directory-files-recursively
     (denote-directory)
     directory-files-no-dot-files-regexp
     :include-directories
     (lambda (f)
       (cond
        ((string-match-p "\\`\\." f) nil)
        ((string-match-p "/\\." f) nil)
        ((denote--exclude-directory-regexp-p f) nil)
        ((file-readable-p f))
        (t)))
     :follow-symlinks))

(defun denote-directory-files ()
  "Return list of absolute file paths in variable `denote-directory'.

@@ -726,17 +748,7 @@ value, as explained in its doc string."
   (seq-remove
    (lambda (f)
      (not (denote-file-has-identifier-p f)))
    (directory-files-recursively
     (denote-directory)
     directory-files-no-dot-files-regexp
     :include-directories
     (lambda (f)
       (cond
        ((when-let ((regexp denote-excluded-directories-regexp))
           (not (string-match-p regexp f))))
        ((file-readable-p f))
        (t)))
     :follow-symlinks))))
    (denote--directory-all-files-recursively))))

(defun denote-directory-text-only-files ()
  "Return list of text files in variable `denote-directory'.
@@ -750,7 +762,7 @@ Filter `denote-directory-files' using `denote-file-is-note-p'."

(defun denote-directory-subdirectories ()
  "Return list of subdirectories in variable `denote-directory'.
Omit dotfiles (such as .git) unconditionally.  Also exclude
Omit dotfiles (such as .git) unconditionally. Also exclude
whatever matches `denote-excluded-directories-regexp'."
  (seq-remove
   (lambda (filename)
@@ -758,9 +770,8 @@ whatever matches `denote-excluded-directories-regexp'."
       (or (not (file-directory-p filename))
           (string-match-p "\\`\\." rel)
           (string-match-p "/\\." rel)
           (when-let ((regexp denote-excluded-directories-regexp))
             (string-match-p regexp rel)))))
   (directory-files-recursively (denote-directory) ".*" t t)))
           (denote--exclude-directory-regexp-p rel))))
   (denote--directory-all-files-recursively)))

(define-obsolete-function-alias
  'denote--subdirs
-- 
2.34.1
Details
Message ID
<87zg75q4er.fsf@protesilaos.com>
In-Reply-To
<20230414200616.5432-1-graham@mgmarlow.com> (view parent)
DKIM signature
missing
Download raw message
> From: Graham Marlow <graham@mgmarlow.com>
> Date: Fri, 14 Apr 2023 13:06:16 -0700

> [... 14 lines elided]

>  denote.el | 41 ++++++++++++++++++++++++++---------------
>  1 file changed, 26 insertions(+), 15 deletions(-)

> [... 74 lines elided]

Thank you!  Perhaps Wade Mealing can try this patch to check if it works
as expected in that setup?

-- 
Protesilaos Stavrou
https://protesilaos.com
Details
Message ID
<a29c4b8c-f4a9-ad7e-9fc6-8123f2b890a3@mgmarlow.com>
In-Reply-To
<87zg75q4er.fsf@protesilaos.com> (view parent)
DKIM signature
missing
Download raw message
> Thank you!  Perhaps Wade Mealing can try this patch to check if it works
> as expected in that setup?

For myself I was able to reproduce the problem in my local testing by 
cloning one or two node repositories into my denote directory and 
invoking denote-directory-subdirectories.

|cd ~/denote/source|
|npx create-react-app my-app1|
|npx create-react-app my-app2|

||

|| Before the call to denote-directory-subdirectories takes 5-8 seconds. 
After applying the changes in this patch it's instantaneous. Likewise 
with denote-subdirectory.
Wade Mealing <wmealing@gmail.com>
Details
Message ID
<CAO4UgPQtxhhqW0tB7eZnVh4nF9vLvnVGx+5oB_78_dg32URSLA@mail.gmail.com>
In-Reply-To
<a29c4b8c-f4a9-ad7e-9fc6-8123f2b890a3@mgmarlow.com> (view parent)
DKIM signature
missing
Download raw message
Apologies, I had replied in html mode and didnt notice.

This significantly improves the performance, from an 8-10 second (with
warm cache), to near instantaneous.

I'm very happy with that.

On Wed, Apr 19, 2023 at 3:04 PM Graham Marlow <graham@mgmarlow.com> wrote:
>
> > Thank you!  Perhaps Wade Mealing can try this patch to check if it works
> > as expected in that setup?
>
> For myself I was able to reproduce the problem in my local testing by
> cloning one or two node repositories into my denote directory and
> invoking denote-directory-subdirectories.
>
> |cd ~/denote/source|
> |npx create-react-app my-app1|
> |npx create-react-app my-app2|
>
> ||
>
> || Before the call to denote-directory-subdirectories takes 5-8 seconds.
> After applying the changes in this patch it's instantaneous. Likewise
> with denote-subdirectory.
>


-- 
Wade Mealing
Details
Message ID
<87pm802md0.fsf@protesilaos.com>
In-Reply-To
<CAO4UgPQtxhhqW0tB7eZnVh4nF9vLvnVGx+5oB_78_dg32URSLA@mail.gmail.com> (view parent)
DKIM signature
missing
Download raw message
> From: Wade Mealing <wmealing@gmail.com>
> Date: Wed, 19 Apr 2023 17:41:34 +1000
>
> Apologies, I had replied in html mode and didnt notice.
>
> This significantly improves the performance, from an 8-10 second (with
> warm cache), to near instantaneous.
>
> I'm very happy with that.

Very well!  I installed the patch and pushed the changes.  Thank you
both for the feedback!

-- 
Protesilaos Stavrou
https://protesilaos.com
Reply to thread Export thread (mbox)