Subject: Re: bug#44254: Performance of package input rewriting

From: Ludovic Courtès <[email-masked]>
Date: Sat, 31 Oct 2020 11:27:06 +0100

Hi Lars,

Lars-Dominik Braun <[email-masked]> skribis:

>> Another thing to look at is the <package> object graph (as show by ‘guix
>> graph’).  Input rewriting can duplicate parts of the graph, which in
>> turn defeats package->derivation memoization.  Just looking at the
>> number of nodes in the graph can give hints.
> Aha, it’s 913 nodes without rewriting, 13916 with rewriting (#:deep? #t) and
> 4286 with rewriting (#:deep? #f) as determined by a rather ad-hoc `guix graph
> -L . -t package python-jupyterlab | grep 'shape = box' | wc -l`. That seems way
> too much. Does that mean I’m using package rewriting in the wrong way or is
> that a bug?

It could be a mixture thereof.  :-)

I guess it’s easy to end up creating huge object graphs.  Here’s an
example of an anti-pattern:

  (define a
    ((package-input-rewriting x) ((package-input-rewriting y) p1))) 

  (define b
    ((package-input-rewriting x) ((package-input-rewriting y) p2)))

The correct use is:

  (define transform
    (package-input-rewriting (append x y)))

  (define a (transform p1))
  (define b (transform p2))

That guarantees that ‘a’ and ‘b’ share most of the nodes of their object
graph.

From a quick look, the code in Guix-Science seemed to be following the
pattern above.

For example, there’s:

--8<---------------cut here---------------start------------->8---
(define python-ipykernel-5.3-bootstrap
  (let ((rewritten ((package-input-rewriting
    `((,python-jupyter-client . ,python-jupyter-client-6.1-bootstrap)
     ;; Indirect through IPython.
     (,python-testpath . ,python-testpath-0.4)
     (,python-nbformat . ,python-nbformat-5.0)))
   python-ipykernel-5.3-proper)))
    (package
      (inherit rewritten)
      (name "python-ipykernel-bootstrap"))))

(define-public python-jupyter-client-6.1
  ((package-input-rewriting
   `((,python-ipykernel . ,python-ipykernel-5.3-bootstrap)
     (,python-jupyter-core . ,python-jupyter-core-4.6)
     ;; Indirect through IPython.
     (,python-testpath . ,python-testpath-0.4)
     (,python-nbformat . ,python-nbformat-5.0)))
   python-jupyter-client-6.1-proper))

(define-public python-ipykernel-5.3
  ((package-input-rewriting
   `((,python-jupyter-client . ,python-jupyter-client-6.1)
     ;; Indirect through IPython.
     (,python-testpath . ,python-testpath-0.4)
     (,python-nbformat . ,python-nbformat-5.0)))
   python-ipykernel-5.3-proper))
--8<---------------cut here---------------end--------------->8---

It seems to me that you’re redefining a dependency graph, node by node.
Thus, you probably don’t need ‘package-input-rewriting’ here.  What you
did in Guix-Science commit 972795a23cc9eb5a0bb1a2ffb5681d151fc4d4b0
looks more appropriate to me, in terms of style and semantics.

Does that make sense?

Thanks,
Ludo’.