Subject: Re: [PATCH v4] Optimise search-patch (reducing I/O)

From: Maxime Devos <[email-masked]>
Date: Thu, 23 Sep 2021 19:26:51 +0200

Ludovic Courtès schreef op di 21-09-2021 om 18:55 [+0200]:
> Hi!
> 
> I took the liberty to reopen this patch because there were good ideas
> IMO.  I’m sorry if my many questions and lack of responsiveness came out
> as a suggestion that this approach wasn’t good.

I reordered your mail a little.

> Looking at the big picture, what I’d like to have is a package
> derivation cache designed in such a way that “guix install foo” wouldn’t
> even need to load any package module on a cache hit.  That’d make a
> noticeable difference performance-wise, that’s another level of
> complexity…  (I have a rough design in mind that we could discuss.)

This ‘package derivation cache’ seems an interesting idea to pursue,
though I wonder what could be used as ‘keys’ in the derivation cache.

Package names aren't sufficient because packages can have multiple versions,
package names + package versions aren't sufficient because packages can have
multiple variants.  Grafts might need some care.  Having multiple versions of
guix can be addressed by including the commits of every channel in the key.

Even if ‘foo’ isn't in the cache, the cache can still be useful if the
inputs ‘bar’ and ‘baz’ of foo are in the cache.

> Ludovic Courtès <[email-masked]> skribis:
> 
> > > +;; repeated 'stat' calls.  Allow computing the hash of the file in advance,
> > > +;; to avoid having to send the file to the daemon when it is already interned
> > > +;; in the store.
> > >  (define-record-type <local-file>
> > > -  (%%local-file file absolute name recursive? select?)
> > > +  (%%local-file file absolute name sha256 recursive? select?)
> > >    local-file?
> > >    (file       local-file-file)                    ;string
> > >    (absolute   %local-file-absolute-file-name)     ;promise string
> > >    (name       local-file-name)                    ;string
> > > +  (sha256     local-file-sha256)                  ;sha256 bytevector | #f
> > 
> > Could we store the result of ‘fixed-output-path’ rather than the SHA256,
> > while we’re at it?

Embedding the result of ‘fixed-output-path’ in the .go might be problematic
from a closure size perspective, as that would create additional references in the
store items of guix.

> I tried that with the patch below, roughly taking the same approach as
> your patch series, but somewhat simplified, mostly so I could
> experiment. [...]
> 
> We can estimate the performance of that strategy by commenting out the
> ‘add-temp-root*’ call (thus getting a single RPC) in
> ‘local-file-compiler’: this time it’s slightly faster, but we’re in the
> 1% range on the wall-clock time of ‘guix build pigx -d --no-grafts’:
> [...]
> Perhaps the gains would be a bit higher if we change all the package
> files to use ‘local-patches’, but we probably can’t expect a lot more
> anyway since that process is CPU-bound.
> 
> So I don’t know.  It feels like a worthy optimization, and one that’s
> manageable from a maintenance viewpoint, but it buys us very little.
> 
> Thoughts?

As it is only <1%, I would prefer trying the ‘package derivation cache’
first, as it seems to have more potential.

> Ludo’.