[PATCH 0/7] Memoize packages associated with cross building.

This comes from dumping the contents of the store connection object

cache after computing derivations targeting i586-pc-gnu, and looking at

what packages appear multiple times as cache keys.

Christopher Baines (7):

gnu: Memozise make-ld-wrapper results.

gnu: Memozise cross-binutils results.

gnu: Memozise cross-gcc results.

gnu: Memozise cross-kernel-headers results.

gnu: Memozise cross-mig results.

gnu: Memozise cross-libc results.

packages: rust: Memoize make-rust-sysroot results.

gnu/packages/base.scm | 126 +++---

gnu/packages/cross-base.scm | 804 +++++++++++++++++++-----------------

gnu/packages/rust.scm | 6 +-

3 files changed, 486 insertions(+), 450 deletions(-)

To ensure that it just returns a single package record for some given

arguments, as this helps to avoid poor performance of the store connection

object cache.

make-ld-wrapper/implementation and call it.

(make-ld-wrapper/implementation) New procedure.

Change-Id: Id6fc805a4a7ffbc5ff0a5174eafcdf2c7c46854d

---

gnu/packages/base.scm | 126 ++++++++++++++++++++++--------------------

1 file changed, 66 insertions(+), 60 deletions(-)

To ensure that it just returns a single package record for some given

arguments, as this helps to avoid poor performance of the store connection

object cache.

cross-binutils/implementation and call it.

(cross-binutils/implementation) New procedure.

(cross-binutils/deprecated): Call cross-binutils/implementation.

Change-Id: Ic7a493177026c7a699108ab6d75482ff3c189340

---

gnu/packages/cross-base.scm | 92 +++++++++++++++++++------------------

1 file changed, 48 insertions(+), 44 deletions(-)

To ensure that it just returns a single package record for some given

arguments, as this helps to avoid poor performance of the store connection

object cache.

cross-gcc/implementation and call it.

(cross-gcc/implementation) New procedure.

Change-Id: Ibeafaa4d652fc1d6fd27870b82a309c177b66a05

---

gnu/packages/cross-base.scm | 183 +++++++++++++++++++-----------------

1 file changed, 95 insertions(+), 88 deletions(-)

To ensure that it just returns a single package record for some given

arguments, as this helps to avoid poor performance of the store connection

object cache.

cross-libc/implementation and call it.

(cross-libc/implementation) New procedure.

Change-Id: I72f430136860e5d1fd9edeb3274678186b896bd4

---

gnu/packages/cross-base.scm | 165 +++++++++++++++++++-----------------

1 file changed, 85 insertions(+), 80 deletions(-)

To ensure that it just returns a single package record for some given

arguments, as this helps to avoid poor performance of the store connection

object cache.

make-rust-sysroot/implementation.

(make-rust-sysroot/implementation): New variable.

Change-Id: Ibb30c7398328c87c032bb8828635a34ada935167

---

gnu/packages/rust.scm | 6 +++++-

1 file changed, 5 insertions(+), 1 deletion(-)

To ensure that it just returns a single package record for some given

arguments, as this helps to avoid poor performance of the store connection

object cache.

cross-mig/implementation and call it.

(cross-mig/implementation) New procedure.

Change-Id: Iaf2a69c48664d2f0766b9d2f6e981653e0e3c44c

---

gnu/packages/cross-base.scm | 56 +++++++++++++++++++------------------

1 file changed, 29 insertions(+), 27 deletions(-)

To ensure that it just returns a single package record for some given

arguments, as this helps to avoid poor performance of the store connection

object cache.

cross-kernel-headers/implementation and call it.

(cross-kernel-headers/implementation) New procedure.

Change-Id: I345604c089e7a8a9884c07f39c95f960760e86db

---

gnu/packages/cross-base.scm | 306 ++++++++++++++++++------------------

1 file changed, 157 insertions(+), 149 deletions(-)

Hi,

Christopher Baines <mail@cbaines.net> skribis:

Do you have figures before and after the change?

The reason I’m asking is that (gnu packages commencement) arranges to

not call ‘make-ld-wrapper’ repeatedly already. For instance, there’s:

(define-public ld-wrapper

;; The final 'ld' wrapper, which uses the final Guile and Binutils.

(make-ld-wrapper "ld-wrapper"

#:binutils binutils-final

#:guile guile-final

#:bash bash-final))

and from there on we manipulate a single <package> record.

Thanks,

Ludo’.

Ludovic Courtès <ludo@gnu.org> writes:

I believe the reason packages from make-ld-wrapper were showing up

multiple times in the cache for me is linked to it's use in the

cross-base module, as part of the cross-gcc procedure.

A later commit does change cross-gcc to return a single package record

for some given arguments, so that probably resolves the biggest misuse

of make-ld-wrapper.

I think there's other cases (in the llvm and mold modules) where it

looks like it's called multiple times with the same arguments, so maybe

that's an argument for having memoization around make-ld-wrapper even

though it's not needed for all uses.

Hi,

Christopher Baines <mail@cbaines.net> skribis:

Oh, I see.

Before resorting to memoization (which doesn’t come for free), we can

for instance make sure we do not create new package records from inputs

fields (since they are thunked, a fresh record would be created each

time the field is accessed). Here’s an example:

This particular one doesn’t have a huge impact on simple cases but it

might pay off for bigger graphs:

On guix-devel, Efraim mentioned cargo-build-system packages. Is that

related?

Thanks for looking into this!

Ludo’.

Ludovic Courtès <ludo@gnu.org> writes:

Looking at the various changes, while rust-sysroot doesn't appear that

many times in the cache, I think it's responsible for most of the

slowness. Maybe that's due in part to what you say above, using the

cross- procedures in the native-inputs field.

Maybe the transformation you suggest above could be applied, but I'd be

worried about how that would work if you set one

%current-system/%current-target-system when you call the procedure, then

another when you compute the derivation.

This is intended to help with spotting duplication in the object cache, so

where many keys, for example package records map to the same derivation. This

represents an opportunity for improved performance if you can reduce this

duplication in the cache, and better take advantage of the already present

cache entries.

I'm thinking this can be used by the data service, but maybe it could also be

worked in to guix commands.

Change-Id: Ia6c816f871d10cae6807543224250110099d764f

---

guix/store.scm | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++

1 file changed, 59 insertions(+)

Hi,

Christopher Baines <mail@cbaines.net> skribis:

[...]

We should avoid using ‘mlambda’ (without ‘q’) with packages as it leads

to deep object comparisons. That’s why for packages we typically

always have one-argument (mlambdaq (package) …).

But since ‘base-rust’ wasn’t a parameter before, let’s keep it simple

(‘diff --ignore-space-change’):

It should help when cross-compiling several ‘cargo-build-system’ packages:

(Here we removed ~2.5k nodes from the cache.)

WDYT?

Ludo’.

Christopher Baines <mail@cbaines.net> skribis:

Another way to detect this is by looking at ‘add-data-to-store-cache’

stats:

Ideally, the hit rate there would be 0% and we could remove it.

If there’s a positive hit rate, it means we keep adding the same .drv

and/or *-builder files to the store, meaning that the object cache was

ineffective.

Do you have an example output of this?

How helpful does it look to you in practice?

I guess it we’d need to add a ‘register-profiling-hook!’ call somewhere?

If we were to add this, I’d suggest adding a docstring and following the

same style as the rest of the code (no long identifiers for local

variables and no car/cdr, for example).

Thanks,

Ludo’.

Ludovic Courtès <ludo@gnu.org> writes:

...

Yeah, that does look good. I pushed my earlier version of this patch

this morning though.

I did have a look at trying to adapt the changes to fit in (guix

build-system cargo) instead, as I noticed that seemed to be a pattern

elsewhere, but I think there's something weird going on with the use of

make-rust-sysroot there since default-rust-sysroot takes an argument,

but doesn't use it. Maybe once that's figured out, we can move the

memoization there and switch to just using the target as the key.

Unfortunately I'm still waiting to see what effect this has on the data

service processing revisions. I'm pretty sure it's going to help, but

I'm concerned it's not going to help enough to make processing revisions

for patches feasible again.

Ludovic Courtès <ludo@gnu.org> writes:

Yep, so here's some output I get from computing all the cross

derivations to i586-pc-gnu:

value #<derivation

/gnu/store/lqxlksh00cshc816xqfq47r3jjdfj2p9-subversion-1.14.2.drv =>

/gnu/store/92avphfdcrcaxx8m5a6ihmw558bj3np8-subversion-1.14.2 7fcfbe5f9280>

cached 4174 times

example keys:

- #<package subversion@1.14.2 gnu/packages/version-control.scm:2316 7fcfc1c924d0>

- #<package subversion@1.14.2 gnu/packages/version-control.scm:2316 7fcfc1c924d0>

- #<file-append #<package subversion@1.14.2 gnu/packages/version-control.scm:2316 7fcfc1c924d0> "/bin/svn">

- #<file-append #<package subversion@1.14.2 gnu/packages/version-control.scm:2316 7fcfc1c924d0> "/bin/svn">

So it's not immediately obvious what the issue is, but if you search for

"/bin/svn", then you find the file-append calls in svn-fetch, so there's

a couple of new file-append records added to the cache for each

svn-fetch.

value /gnu/store/qi1km3qlv5hdsql6h3ibwvz6v4z8lqbz-ld-wrapper.in cached 755 times

example keys:

- #<<local-file> file: "/home/chris/Projects/Guix/guix/gnu/packages/ld-wrapper.in" absolute: #<promise "/home/chris/Projects/Guix/guix/gnu/packages/ld-wrapper.in"> name: "ld-wrapper.in" recursive?: #t select?: #<procedure true (file stat)>>

- #<<local-file> file: "/home/chris/Projects/Guix/guix/gnu/packages/ld-wrapper.in" absolute: #<promise "/home/chris/Projects/Guix/guix/gnu/packages/ld-wrapper.in"> name: "ld-wrapper.in" recursive?: #t select?: #<procedure true (file stat)>>

- #<<local-file> file: "/home/chris/Projects/Guix/guix/gnu/packages/ld-wrapper.in" absolute: #<promise "/home/chris/Projects/Guix/guix/gnu/packages/ld-wrapper.in"> name: "ld-wrapper.in" recursive?: #t select?: #<procedure true (file stat)>>

- #<<local-file> file: "/home/chris/Projects/Guix/guix/gnu/packages/ld-wrapper.in" absolute: #<promise "/home/chris/Projects/Guix/guix/gnu/packages/ld-wrapper.in"> name: "ld-wrapper.in" recursive?: #t select?: #<procedure true (file stat)>>

Similar to file-append, it's not immediately obvious where all these

local-file's are coming from, but searching for ld-wrapper.in suggests

make-ld-wrapper.

My thinking here is that maybe it's worth not caching everything in the

object cache. Particularly for file-append, I'm not sure what the cache

is actually doing. As for local-file, given there's lower level caching,

maybe it's worth leaning on that and not caching local-file either. I

tried this out, and it seemed to not make performance worse at least,

and it did remove the duplication from the cache.

Other things that show up include:

value #<derivation /gnu/store/z28jfsm43wg50cvdfq0548pbf5jfhk8r-binutils-cross-i586-pc-gnu-2.38.drv => /gnu/store/lkmwq399jllig2a5r323v6y9nflpn6gn-binutils-cross-i586-pc-gnu-2.38 7fcfbd358000> cached 1228 times

example keys:

- #<package binutils-cross-i586-pc-gnu@2.38 gnu/packages/cross-base.scm:79 7fcfbf3bb8f0>

- #<package binutils-cross-i586-pc-gnu@2.38 gnu/packages/cross-base.scm:79 7fcfbf3b6630>

- #<package binutils-cross-i586-pc-gnu@2.38 gnu/packages/cross-base.scm:79 7fcfbf3b6420>

- #<package binutils-cross-i586-pc-gnu@2.38 gnu/packages/cross-base.scm:79 7fcfbf3b62c0>

On Fri, Jan 12, 2024 at 05:57:26PM +0000, Christopher Baines wrote:

I looked at the build system a bit and I think it was a combination of

cargo-culting the other cross build implementations that took a target

argument for the cross-compilers and I figured that rust-sysroot would

also need one. The other bit was I think I had in mind the possibility

of choosing seemingly arbitrary targets which were supported by rust but

not known to Guix and having it possible to cross-compile to those.

Given that rust, like go, IIRC doesn't actually need a cross-compiled

compiler to build cross-compiled packages, could it be that parts of

that logic can be rewritten/simplified?

--

Efraim Flashner <efraim@flashner.co.il> ????? ?????

GPG key = A28B F40C 3E55 1372 662D 14F7 41AA E7DC CA3D 8351

Confidentiality cannot be guaranteed on emails sent or received unencrypted

Hello,

Christopher Baines <mail@cbaines.net> skribis:

[...]

What matters is that ‘make-rust-sysroot’ takes a single argument,

‘target’, so we can safely write:

(define make-rust-sysroot

(mlambda (target)

…))

I think it’s important to not bring a deep <package> comparison because

this kind of cost is then hard to pinpoint.

Perhaps you can adjust ‘make-rust-sysroot’ along these lines?

Thanks,

Ludo’.