[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#43159] [PATCH 1/2] scripts: Use 'define-command' and have 'guix hel
From: |
Ludovic Courtès |
Subject: |
[bug#43159] [PATCH 1/2] scripts: Use 'define-command' and have 'guix help' use that. |
Date: |
Thu, 03 Sep 2020 15:41:41 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) |
Hi Maxim,
Maxim Cournoyer <maxim.cournoyer@gmail.com> skribis:
> Ludovic Courtès <ludo@gnu.org> writes:
[...]
>> +;; Syntactic keywords.
>> +(define synopsis 'command-synopsis)
>> +(define category 'command-category)
>
> Are these definition really necessary/useful? I would have thought
> having category and synopsis understood as literals in the
> define-command syntax was enough?
It’s not strictly necessary but it’s been considered “good practice”.
That allows users to detect name clashes, to rename/hide/etc. syntactic
keywords and so on.
>> +(define-syntax define-command
>> + (syntax-rules (category synopsis)
[...]
>> -(define (guix-archive . args)
>> +(define-command (guix-archive . args)
>> + (category advanced)
>
> It'd be helpful if the category was an enum to keep the set of
> categories focused and helpful.
Yes, I thought about making it a syntactic keyword; let’s see.
>> + ;; The strategy here is to parse FILE. This is much cheaper than a
>> + ;; technique based on run-time introspection where we'd load FILE and all
>> + ;; the modules it depends on.
>
> Interesting! Have you measure it? I would have thought loading a couple
> optimized byte code modules could have been nearly as fast as parsing
> files manually. If so, I think it'd be preferable to use introspection
> rather than implement a custom parser.
On a fast recent laptop with an SSD, a load of 0, hot cache, etc., we’d
still be below 1s. But see:
--8<---------------cut here---------------start------------->8---
$ strace -c guix help >/dev/null
% time seconds usecs/call calls errors syscall
------ ----------- ----------- --------- --------- ------------------
62.69 0.002698 1 2266 2043 stat
10.94 0.000471 2 161 2 lstat
4.55 0.000196 0 246 mmap
4.51 0.000194 0 330 172 openat
[...]
------ ----------- ----------- --------- --------- ------------------
100.00 0.004304 1 3748 2235 total
$ strace -c guile -c '(use-modules (guix scripts system) (guix scripts
authenticate))'
% time seconds usecs/call calls errors syscall
------ ----------- ----------- --------- --------- ------------------
54.27 0.007799 1 5735 4518 stat
12.00 0.001724 11 149 27 futex
9.06 0.001302 0 1328 651 openat
7.24 0.001040 1 822 mmap
[...]
------ ----------- ----------- --------- --------- ------------------
100.00 0.014371 1 10334 5202 total
--8<---------------cut here---------------end--------------->8---
(The 1st run is the current ‘guix help’; the 2nd run +/- emulates what
you propose.)
Loading all the modules translates into a lot more I/O, roughly an order
of magnitude. We’re talking about loading tens of modules just to get
at that synopsis:
--8<---------------cut here---------------start------------->8---
scheme@(guile-user)> ,use(guix modules)
scheme@(guile-user)> (length (source-module-closure '((guix scripts system)
(guix scripts authenticate))))
$10 = 439
scheme@(guile-user)> (length (source-module-closure '((guix scripts) (guix
ui))))
$11 = 31
--8<---------------cut here---------------end--------------->8---
Memory usage would also be very different:
--8<---------------cut here---------------start------------->8---
$ \time guix help >/dev/null
0.07user 0.01system 0:00.06elapsed 128%CPU (0avgtext+0avgdata 35348maxresident)k
0inputs+0outputs (0major+3906minor)pagefaults 0swaps
$ \time guile -c '(use-modules (guix scripts system) (guix scripts
authenticate))'
0.42user 0.05system 0:00.37elapsed 128%CPU (0avgtext+0avgdata
166916maxresident)k
0inputs+0outputs (0major+15148minor)pagefaults 0swaps
--8<---------------cut here---------------end--------------->8---
In summary, while this approach undoubtedly looks awkward to any Lisper,
I think it’s a good way to not contribute to the general impression of
sluggishness and resource-hungriness of ‘guix’ commands. :-)
>> + (define (display-commands commands)
>> + (let* ((names (map (lambda (command)
>> + (string-join (command-name command)))
>> + commands))
>> + (max-width (reduce max 0 (map string-length names))))
>
> You can drop reduce and use (max (map string-length names)) instead.
I could do (apply max (map …)) but I don’t like the idea of abusing
variadic argument lists in that way—I know, it’s very subjective. ;-)
Thanks for your feedback, I’ll send a v2!
Ludo’.
[bug#43159] [PATCH 0/2] Make 'guix help' helpful, Efraim Flashner, 2020/09/02
[bug#43159] [PATCH 0/2] Make 'guix help' helpful, zimoun, 2020/09/03