Subject: [bug#50384] [PATCH] Optimise search-patch (reducing I/O)

From: Ludovic Courtès <[email-masked]>
Date: Sun, 05 Sep 2021 00:04:25 +0200

Maxime Devos <[email-masked]> skribis:

> +(define-syntax search-patch
> +  (lambda (s)
> +    "Search the patch FILE-NAME and compute its hash at expansion time
> +if possible.  Return #f if not found."
> +    (syntax-case s ()
> +      ((_ file-name)
> +       (string? (syntax->datum #'file-name))
> +       ;; FILE-NAME is a constant string, so the hash can be computed
> +       ;; in advance.
> +       (let ((patch (try-search-patch (syntax->datum #'file-name))))
> +         (if patch
> +             #`(%local-patch-file file-name #,(file-hash* patch #:select? true))
> +             (begin
> +               (warning (source-properties->location
> +                         (syntax-source #'file-name))
> +                        (G_ "~a: patch not found at expansion time")
> +                        (syntax->datum #'ile-name))
> +               #'(%search-patch file-name)))))
> +      ;; FILE-NAME is variable, so the hash cannot be pre-computed.
> +      ((_ file-name) #'(%search-patch file-name))
> +      ;; search-patch is being used used in a construct like
> +      ;; (map search-patch ...).
> +      (id (identifier? #'id) #'%search-patch))))

It’s clever… but also a bit evil, in that it changes the semantics of
package files in a surprising way.  Modifying foo.patch without
recompiling foo.scm would lead you to still use the old foo.patch, which
can be rather off-putting and error-prone IMO.

To address this, ‘local-file’ could store the inode/mtime + computed
store file name (rather than the SHA256).  ‘local-file-compiler’ would
check whether the actual file has matching inode/mtime before returning
the computed store file name.  Problem is that inode/mtime are
guaranteed to differ once you’ve run “make install”.  :-/

Intuitively, I’d have imagined a cache populated at run time; it would
map, say, file name/inode/mtime to a store file name.  ‘add-to-store’
(or some wrapper above it) would check the cache and return the store
file name directly, unless ‘valid-path?’ says it no longer exists.
Downside is that this would be a per-user cache and you’d still pay the
cost until it’s warm.  Advantage is that you could easily tell whether
it’s stale.

Thoughts?

Ludo’.