Subject: [bug#68266] [PATCH 7/7] packages: rust: Memoize make-rust-sysroot results.

From: Christopher Baines <[email-masked]>
Date: Fri, 12 Jan 2024 17:57:26 +0000

Ludovic Courtès <[email-masked]> writes:

> Christopher Baines <[email-masked]> skribis:
>
>> 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.
>>
>> * gnu/packages/rust.scm (make-rust-sysroot): Move code to
>> make-rust-sysroot/implementation.
>> (make-rust-sysroot/implementation): New variable.
>>
>> Change-Id: Ibb30c7398328c87c032bb8828635a34ada935167
>
> [...]
>
>>  (define*-public (make-rust-sysroot target)
>> -  (let ((base-rust rust))
>> +  (make-rust-sysroot/implementation target rust))
>> +
>> +(define make-rust-sysroot/implementation
>> +  (mlambda (target base-rust)
>>      (package
>>        (inherit base-rust)
>>        (name (string-append "rust-sysroot-for-" target))
>
> 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’):

...

> WDYT?

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.