GNU bug report logs - #79458
[PATCH] Call load-path-filter-function in Flocate_file_internal

Previous Next

Full log

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: 79458 <at> debbugs.gnu.org
Subject: Re: bug#79458: [PATCH] Call load-path-filter-function in
 Flocate_file_internal
Date: Tue, 16 Sep 2025 17:02:45 -0400

> From 30d3bdf9d6da9c77d529a292c0abe9f48be8d57b Mon Sep 17 00:00:00 2001
> From: Spencer Baugh <sbaugh <at> janestreet.com>
> Date: Tue, 16 Sep 2025 12:09:54 -0400
> Subject: [PATCH] Call load-path-filter-function in Flocate_file_internal
>
> This allows calls like (locate-library "term/xterm") made by
> tty-run-terminal-initialization to be optimized for long
> load-paths.
>
> * lisp/startup.el (load-path-filter-cache-directory-files):
> Filter on the directory of multi-component file names, and
> handle nil PREFIXES.
> * src/lread.c (Flocate_file_internal): Call
> load-path-filter-function.
> * test/lisp/startup-tests.el
> (startup-tests/load-path-filter-cache-directory-files): Add.

s/PREFIXES/SUFFIXES/?

> ---
>  lisp/startup.el            | 52 +++++++++++++++++++++-----------------
>  src/lread.c                |  2 ++
>  test/lisp/startup-tests.el | 12 +++++++++
>  3 files changed, 43 insertions(+), 23 deletions(-)
>
> diff --git a/lisp/startup.el b/lisp/startup.el
> index 836ead6deb0..5d824a0966e 100644
> --- a/lisp/startup.el
> +++ b/lisp/startup.el
> @@ -1157,38 +1157,44 @@ load-path-filter-cache-directory-files
>  PATH should be a list of directories such as `load-path'.
>  Returns a copy of PATH with any directories that cannot contain FILE
>  with SUFFIXES removed from it.
> -Doesn't filter PATH if FILE is an absolute file name or if FILE is
> -a relative file name with leading directories.
> +Doesn't filter PATH if FILE is an absolute file name.
>  
>  Caches contents of directories in `load-path-filter--cache'.
>  
>  This function is called from `load' via `load-path-filter-function'."
> -  (if (file-name-directory file)
> -      ;; FILE has more than one component, don't bother filtering.
> +  (if (or (file-name-absolute-p file)
> +          (string-empty-p file)
> +          (null suffixes)
> +          ;; Don't bother filtering if "" is among the suffixes.
> +          ;; It's a much less common use-case and it would use
> +          ;; more memory to keep the corresponding info.
> +          (member "" suffixes))
>        path
>      (pcase-let
>          ((`(,rx . ,ht)
>            (with-memoization (alist-get suffixes load-path-filter--cache
>                                         nil nil #'equal)
> -            (if (member "" suffixes)
> -                '(nil ;; Optimize the filtering.
> -                  ;; Don't bother filtering if "" is among the suffixes.
> -                  ;; It's a much less common use-case and it would use
> -                  ;; more memory to keep the corresponding info.
> -                  . nil)
> -              (cons (concat (regexp-opt suffixes) "\\'")
> -                    (make-hash-table :test #'equal))))))
> -      (if (null ht)
> -          path
> -        (let ((completion-regexp-list nil))
> -          (seq-filter
> -           (lambda (dir)
> -             (when (file-directory-p dir)
> -               (try-completion
> -                file
> -                (with-memoization (gethash dir ht)
> -                  (directory-files dir nil rx t)))))
> -           path))))))
> +            (cons (concat (regexp-opt (cons "/" suffixes)) "\\'")
> +                  (make-hash-table :test #'equal)))))
> +      (let ((filter-on
> +             ;; Filter on the first component of FILE.
> +             (let ((split (file-name-split file)))
> +               (if (cdr split)
> +                   ;; The first component must be a directory.
> +                   (file-name-as-directory (car split))
> +                 (car split))))
> +            (completion-regexp-list nil)
> +            (completion-ignore-case nil))
> +        (seq-filter
> +         (lambda (dir)
> +           (when (file-directory-p dir)
> +             (try-completion
> +              filter-on
> +              (with-memoization (gethash dir ht)
> +                (seq-filter
> +                 (lambda (file) (string-match rx file))
> +                 (file-name-all-completions "" dir))))))
> +         path)))))

Have you compared the above `string-match` approach to letting
`file-name-all-completions` do the regexp matching via
`completion-regexp-list`?

> --- a/src/lread.c
> +++ b/src/lread.c
> @@ -1601,6 +1601,8 @@ DEFUN ("locate-file-internal", Flocate_file_internal, Slocate_file_internal, 2,
>    (Lisp_Object filename, Lisp_Object path, Lisp_Object suffixes, Lisp_Object predicate)
>  {
>    Lisp_Object file;
> +  if (FUNCTIONP (Vload_path_filter_function))
> +    path = calln (Vload_path_filter_function, path, filename, suffixes);
>    int fd = openp (path, filename, suffixes, &file, predicate, false, true,
>  		  NULL);
>    if (NILP (predicate) && fd >= 0)

Of course, this makes the name `load-path-filter-function` a lie since
it applies to more than just `load-path`.

Have you tried to move the call directly into `openp`?

Also, I wonder what's the impact on the size of the cache.  I get the
impression that we might be more likely to get duplicates because of
slight variations of `suffixes`.


        Stefan

Previous Next

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