Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> From 30d3bdf9d6da9c77d529a292c0abe9f48be8d57b Mon Sep 17 00:00:00 2001
>> From: Spencer Baugh <sbaugh@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/?

Oops, yes, will fix in next version.

>> ---
>>  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`?

Yes.  Alas, completion-regexp-list is applied by
file-name-all-completions *before* the "/" is added to the file names of
directories.  So using completion-regexp-list just didn't work :(

>> --- 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`.

Yes.  We probably should change the name.

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

Just tried, it seems to work fine.  The attached patch does that.

> 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`.

True.

What about just having a single cached list per directory, instead of
having separate lists pre-filtered with suffixes?  That would avoid the
problem of duplication due to different suffixes.  The attached patch
does that (in addition to moving the call to openp).  I especially like
that it now works when "" is present as a suffix.