Subject: Re: [PATCH v4 0/3] Faster cache generation (similar as v3)

From: Ludovic Courtès <[email-masked]>
Date: Sun, 03 May 2020 18:43:41 +0200

Hello!

zimoun <[email-masked]> skribis:

> The aim of this version v4 is to keep the same searching performances as the previous version v3 but to drastically reduce the generation of the cache.  On my laptop, the overhead is now 4 seconds; compared to more than 20 seconds for v2 and v3.
>
> # default
> time guix build /gnu/store/0nfpp82mqglpwvl1nbfpaphw5db2ivcp-guix-package-cache.drv --check
> # v4
> time guix build /gnu/store/y78gfh1n7m3kyrj8wsqj25qc2cbc1a4d-guix-package-cache.drv --check
>
> |      | default  | v4        |
> |------+----------+-----------|
> | real | 0m6.012s | 0m10.244s |
> | user | 0m0.541s | 0m0.542s  |
> | sys  | 0m0.033s | 0m0.032s  |

Not bad!

> In the version v3, the cache is built using 'cons' and 'fold-packages' (wrapper to 'fold-module-public-variables').  The version v4 modifies -- by adding other information -- the function 'generate-package-cache' which uses 'vhash' and 'fold-module-public-variables*'.
>
> Therefore the cache '/lib/guix/package.cache' contains more
> information.

This breaks the binary interface, so we’ll have to analyze the impact of
such a change and devise a strategy.

> (The v4 structure of 'package.cache' is a quick draft, so details
> should be discussed and an interesting move should to have a
> structured (binary and all strings) S-exp; because it should become an
> entry point to export the packages list to JSON.  WDYT?)

It’s on purpose that this cache is an object file: it just needs to be
mmap’d, and that’s it.  It’s the cheapest possible way to do it.
Parsing sexps would be more costly, and since we’re talking about
startup time, this is sensitive.

> Now, we are comparing apples to apples and the cost to compute BM25 (v2) is not free at all.  Remember that BM25 is the state-of-the-art of information retrieval (relevance ranking) and it is delegated to Xapian (v2).  I do not know if there is perfomance bottleneck between Guix, Guile-Xapian and Xapian itself but for sure the computation of BM25 is not free.  More about that soon.
>
> To be clear about BM25 and caching, what I have in mind is:
>   1. "guix search --build-index" optionally done by the user if they wants for example the BM25 ranking.

Something that must be done explicitly doesn’t seem great to me.  As a
user, I’d rather not think about search indexes and all.  But I don’t
know, maybe if it happened automatically on the first ‘guix search’
invocation that’d be fine.

>   2. Use BM25 metrics to detect poor package meta-data (synopsis and description); if it worth why not add another checker to "guix lint".

That’d be interesting!

>  1. The name of 'fold-packages*' should be misleading since it does not return "true" packages.

Did you see ‘fold-available-packages’?  It seems you could extend it
instead of introducing ‘fold-packages*’, no?

>  2. The function 'package->recutils' in 'guix/ui.scm' is modified but it is not the better.
>
>           (match (package-supported-systems p)
>             (('cache supported-systems)
>              (string-join supported-systems))
>             (_
>              (string-join (package-transitive-supported-systems p)))))
>
>     However it avoids to duplicate code; as it is done in version v3.

I made suggestions to Arun’s v3 about the API here.  Essentially, I
think I proposed having a procedure that takes the list of fields as
keyword parameters, and ‘package->recutils’ would just delegate to that.

>  3. Deprecated packages are displayed (bug in v3 too).
>
>  4. Impolite '@@' is used to access the private license construction.

(guix licenses) could provide a ‘string->license’ procedure.

Stopping here for now because I’m sorta drowning in patch review.  :-)

Thanks for exploring this design space, we’re making progress!

Ludo’.