GNU bug report logs - #50384
[PATCH] Optimise search-patch (reducing I/O)

Previous Next

Full log

View this message in rfc822 format

From: Ludovic Courtès <ludo <at> gnu.org>
To: Maxime Devos <maximedevos <at> telenet.be>
Cc: 50384 <at> debbugs.gnu.org
Subject: [bug#50384] [PATCH] Optimise search-patch (reducing I/O)
Date: Sun, 05 Sep 2021 00:04:25 +0200

Maxime Devos <maximedevos <at> telenet.be> 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’.

Previous Next

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