unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
From: "Ludovic Courtès" <ludo@gnu.org>
To: Maxim Cournoyer <maxim.cournoyer@gmail.com>
Cc: 43159@debbugs.gnu.org
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	[thread overview]
Message-ID: <87blinx9ii.fsf@gnu.org> (raw)
In-Reply-To: <87r1rk595p.fsf@gmail.com> (Maxim Cournoyer's message of "Wed, 02 Sep 2020 14:24:34 -0400")

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’.




  reply	other threads:[~2020-09-03 13:42 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-01 20:35 [bug#43159] [PATCH 0/2] Make 'guix help' helpful Ludovic Courtès
2020-09-01 20:41 ` [bug#43159] [PATCH 1/2] scripts: Use 'define-command' and have 'guix help' use that Ludovic Courtès
2020-09-01 20:41   ` [bug#43159] [PATCH 2/2] ui: '--help' output links to <https://guix.gnu.org/help/> Ludovic Courtès
2020-09-02 18:27     ` Maxim Cournoyer
2020-09-02 18:24   ` [bug#43159] [PATCH 1/2] scripts: Use 'define-command' and have 'guix help' use that Maxim Cournoyer
2020-09-03 13:41     ` Ludovic Courtès [this message]
2020-09-11 18:58       ` Maxim Cournoyer
2020-09-13 13:03         ` Ludovic Courtès
2020-09-13 23:33           ` Maxim Cournoyer
2020-09-07 12:56     ` [bug#43159] [PATCHES v2] " Ludovic Courtès
2020-09-10 10:34       ` bug#43159: " Ludovic Courtès
2020-09-10 10:55         ` [bug#43159] " Ricardo Wurmus
2020-09-02  8:06 ` [bug#43159] [PATCH 0/2] Make 'guix help' helpful Efraim Flashner
2020-09-02  9:50   ` Ludovic Courtès
2020-09-02 11:09     ` Efraim Flashner
2020-09-03 16:40 ` zimoun
2020-09-07 12:58   ` Ludovic Courtès

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87blinx9ii.fsf@gnu.org \
    --to=ludo@gnu.org \
    --cc=43159@debbugs.gnu.org \
    --cc=maxim.cournoyer@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).