From: Maxim Cournoyer <maxim.cournoyer@gmail.com>
To: "Ludovic Courtès" <ludo@gnu.org>
Cc: 43159@debbugs.gnu.org
Subject: [bug#43159] [PATCH 1/2] scripts: Use 'define-command' and have 'guix help' use that.
Date: Fri, 11 Sep 2020 14:58:19 -0400 [thread overview]
Message-ID: <878sdg6sz8.fsf@gmail.com> (raw)
In-Reply-To: <87blinx9ii.fsf@gnu.org> ("Ludovic Courtès"'s message of "Thu, 03 Sep 2020 15:41:41 +0200")
Hi Ludovic,
Sorry I couldn't reply faster.
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.
I see! Thank you for explaining.
[...]
>>> + ;; 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:
>
> $ 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
>
>
> (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:
>
> 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
>
> Memory usage would also be very different:
>
> $ \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
Thanks for the detailed measurements! It does indeed seem your approach
is better, especially considering memory usage. Perhaps the commands
could have been moved to dedicated modules not using much dependency at
all so that their closure would have been small hence fast to load, but
keeping the commands definitions local to where they are useful is
definitely a nice property.
> 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. ;-)
Eh, I wonder why? I may be missing something, but if max allows it,
doesn't it mean it's a valid use? Anyway, just curious to know what are
the grounds for this personal preference :-).
> Thanks for your feedback, I’ll send a v2!
Thanks! I'm late, but LGTM, thank you.
Maxim
next prev parent reply other threads:[~2020-09-11 18:58 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
2020-09-11 18:58 ` Maxim Cournoyer [this message]
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=878sdg6sz8.fsf@gmail.com \
--to=maxim.cournoyer@gmail.com \
--cc=43159@debbugs.gnu.org \
--cc=ludo@gnu.org \
/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).