From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp0 ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by ms11 with LMTPS id oNyNCbXyUF9mfAAA0tVLHw (envelope-from ) for ; Thu, 03 Sep 2020 13:42:13 +0000 Received: from aspmx1.migadu.com ([2001:41d0:2:4a6f::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp0 with LMTPS id WANjBbXyUF9NHAAA1q6Kng (envelope-from ) for ; Thu, 03 Sep 2020 13:42:13 +0000 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id 4BF9B9403C8 for ; Thu, 3 Sep 2020 13:42:12 +0000 (UTC) Received: from localhost ([::1]:43100 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kDpVB-0005s5-23 for larch@yhetil.org; Thu, 03 Sep 2020 09:42:09 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:46620) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kDpV4-0005qI-IQ for guix-patches@gnu.org; Thu, 03 Sep 2020 09:42:02 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:50781) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1kDpV4-0000no-9E for guix-patches@gnu.org; Thu, 03 Sep 2020 09:42:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1kDpV4-0007oC-7b for guix-patches@gnu.org; Thu, 03 Sep 2020 09:42:02 -0400 X-Loop: help-debbugs@gnu.org Subject: [bug#43159] [PATCH 1/2] scripts: Use 'define-command' and have 'guix help' use that. Resent-From: Ludovic =?UTF-8?Q?Court=C3=A8s?= Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Thu, 03 Sep 2020 13:42:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 43159 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: Maxim Cournoyer Cc: 43159@debbugs.gnu.org Received: via spool by 43159-submit@debbugs.gnu.org id=B43159.159914051630004 (code B ref 43159); Thu, 03 Sep 2020 13:42:02 +0000 Received: (at 43159) by debbugs.gnu.org; 3 Sep 2020 13:41:56 +0000 Received: from localhost ([127.0.0.1]:34094 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kDpUy-0007nr-0Y for submit@debbugs.gnu.org; Thu, 03 Sep 2020 09:41:56 -0400 Received: from eggs.gnu.org ([209.51.188.92]:36488) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kDpUv-0007ne-Kz for 43159@debbugs.gnu.org; Thu, 03 Sep 2020 09:41:54 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]:37560) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kDpUq-0000m9-Bz; Thu, 03 Sep 2020 09:41:48 -0400 Received: from [2a01:e0a:1d:7270:af76:b9b:ca24:c465] (port=33638 helo=ribbon) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1kDpUp-0007Lv-Vy; Thu, 03 Sep 2020 09:41:48 -0400 From: Ludovic =?UTF-8?Q?Court=C3=A8s?= References: <20200901203520.21103-1-ludo@gnu.org> <20200901204136.21375-1-ludo@gnu.org> <87r1rk595p.fsf@gmail.com> Date: Thu, 03 Sep 2020 15:41:41 +0200 In-Reply-To: <87r1rk595p.fsf@gmail.com> (Maxim Cournoyer's message of "Wed, 02 Sep 2020 14:24:34 -0400") Message-ID: <87blinx9ii.fsf@gnu.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: -2.3 (--) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-Spam-Score: -3.3 (---) X-BeenThere: guix-patches@gnu.org List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+larch=yhetil.org@gnu.org Sender: "Guix-patches" X-Scanner: scn0 Authentication-Results: aspmx1.migadu.com; dkim=none; dmarc=none; spf=pass (aspmx1.migadu.com: domain of guix-patches-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=guix-patches-bounces@gnu.org X-Spam-Score: -1.01 X-TUID: n500xAQymPXN Hi Maxim, Maxim Cournoyer skribis: > Ludovic Court=C3=A8s 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=E2=80=99s not strictly necessary but it=E2=80=99s been considered =E2=80= =9Cgood practice=E2=80=9D. 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=E2=80=99s 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=E2=80= =99d 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 auth= enticate))' % 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 =E2=80=98guix help=E2=80=99; the 2nd run +/- em= ulates what you propose.) Loading all the modules translates into a lot more I/O, roughly an order of magnitude. We=E2=80=99re 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 =3D 439 scheme@(guile-user)> (length (source-module-closure '((guix scripts) (guix = ui)))) $11 =3D 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 35348maxresid= ent)k 0inputs+0outputs (0major+3906minor)pagefaults 0swaps $ \time guile -c '(use-modules (guix scripts system) (guix scripts authenti= cate))' 0.42user 0.05system 0:00.37elapsed 128%CPU (0avgtext+0avgdata 166916maxresi= dent)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=E2=80=99s a good way to not contribute to the general impression= of sluggishness and resource-hungriness of =E2=80=98guix=E2=80=99 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 =E2=80=A6)) but I don=E2=80=99t like the idea of= abusing variadic argument lists in that way=E2=80=94I know, it=E2=80=99s very subje= ctive. ;-) Thanks for your feedback, I=E2=80=99ll send a v2! Ludo=E2=80=99.