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 MGAdG8TIW18yHQAA0tVLHw (envelope-from ) for ; Fri, 11 Sep 2020 18:58:12 +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 cJORFcTIW1+gXwAA1q6Kng (envelope-from ) for ; Fri, 11 Sep 2020 18:58:12 +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 D65CF9403C8 for ; Fri, 11 Sep 2020 18:58:11 +0000 (UTC) Received: from localhost ([::1]:59936 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kGoFM-0000rk-Oq for larch@yhetil.org; Fri, 11 Sep 2020 14:58:08 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:36128) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kGoFG-0000rQ-Gd for guix-patches@gnu.org; Fri, 11 Sep 2020 14:58:02 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:33977) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1kGoFG-0004Ly-85 for guix-patches@gnu.org; Fri, 11 Sep 2020 14:58:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1kGoFG-0006Z6-7N for guix-patches@gnu.org; Fri, 11 Sep 2020 14:58: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: Maxim Cournoyer Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Fri, 11 Sep 2020 18:58: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: Ludovic =?UTF-8?Q?Court=C3=A8s?= Cc: 43159@debbugs.gnu.org Received: via spool by 43159-submit@debbugs.gnu.org id=B43159.159985066825216 (code B ref 43159); Fri, 11 Sep 2020 18:58:02 +0000 Received: (at 43159) by debbugs.gnu.org; 11 Sep 2020 18:57:48 +0000 Received: from localhost ([127.0.0.1]:45523 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kGoF1-0006Ye-Jd for submit@debbugs.gnu.org; Fri, 11 Sep 2020 14:57:47 -0400 Received: from mail-qt1-f195.google.com ([209.85.160.195]:40526) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1kGoEz-0006YR-2E for 43159@debbugs.gnu.org; Fri, 11 Sep 2020 14:57:46 -0400 Received: by mail-qt1-f195.google.com with SMTP id v54so8694432qtj.7 for <43159@debbugs.gnu.org>; Fri, 11 Sep 2020 11:57:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version:content-transfer-encoding; bh=S98b0C0qLfxzZuupt2I0hCvXelK9OimQ31WdE/x/SQM=; b=fEoFEZ/r7q4N//ragKM4+eVEhIFyNRrxneQglbr3so6vCwhojq5f8uYUA2vEPQeBuQ xHyb1EHc0S8Wq9qMufA6hrsPlSB95VOQ+YAhpLfB1PzdSfjBCBsaKjk7Au45dJDvtgc4 o52ilGWO7Ctwbr9Mztc0lR40k1gUbbp6E/qFaeY/kkH/WkBgDros0VVQvA5jyCXTwnwq UfT/loG0zSq6qTqIwSm5saFIS/jZtm7q95KqN1vJ5rU53l5/NlNPPFa2S15wi3/vOZ5a pgdScM7BR6IB0SjfmMfifrCVfpMbSEIb8agqRng4vw/CsEZVUrEA/quf+aC7Ymv2mO7e ctMg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:references:date:in-reply-to :message-id:user-agent:mime-version:content-transfer-encoding; bh=S98b0C0qLfxzZuupt2I0hCvXelK9OimQ31WdE/x/SQM=; b=ooao0+Cy4u1n5G0YxaMblf8fQX6xBHruWr7+CsvbfdKI0aq9IAK3kBiuFJZtiXch2d mxiW95ND2sWT/BmpkGMC9D/0zA3q7klAX1qhpC1LdZbBFvbtKNgHfgcWTm/RzXBuy4TX rWQxQCmJ5TTVgisjtqECwMgd71QbqoS7R1NqVlRAidFEnlBfdDdZ+qvpnAoniwmyGPUY t/Ek9Ulo6CPGgp7s0xTXEIJqVV30oYJY+UZPPfTj1CmmZMpUSPUorF0Ow6OxngKmG0QK 4Mlhn6WXNwHREyE+vuvfKTVknW0bh41w13OvBRoVostwNzeS7CmQIMDvwgt8uuyjb7Tz VIhw== X-Gm-Message-State: AOAM533ORdCi8kIAJWqYcR0kNbF90WtbFXmUGl9FdacnsVplpiYtpi5h hsNvXUZTPhtNByNZeicGX+P5XQdResS6zw== X-Google-Smtp-Source: ABdhPJyEPD/h0vAXrBYCmZKwPxbmFYjCivR0vMznXE2WG+AjBFad6zKMd/UUaV8etB4aqTXukgboBg== X-Received: by 2002:ac8:35dd:: with SMTP id l29mr3274554qtb.94.1599850659169; Fri, 11 Sep 2020 11:57:39 -0700 (PDT) Received: from hurd (dsl-10-146-200.b2b2c.ca. [72.10.146.200]) by smtp.gmail.com with ESMTPSA id w59sm3837440qtd.1.2020.09.11.11.57.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 11 Sep 2020 11:57:38 -0700 (PDT) From: Maxim Cournoyer References: <20200901203520.21103-1-ludo@gnu.org> <20200901204136.21375-1-ludo@gnu.org> <87r1rk595p.fsf@gmail.com> <87blinx9ii.fsf@gnu.org> Date: Fri, 11 Sep 2020 14:58:19 -0400 In-Reply-To: <87blinx9ii.fsf@gnu.org> ("Ludovic =?UTF-8?Q?Court=C3=A8s?="'s message of "Thu, 03 Sep 2020 15:41:41 +0200") Message-ID: <878sdg6sz8.fsf@gmail.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Score: 0.0 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-Spam-Score: -1.0 (-) 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=fail (rsa verify failed) header.d=gmail.com header.s=20161025 header.b=fEoFEZ/r; dmarc=fail reason="SPF not aligned (relaxed)" header.from=gmail.com (policy=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: 0.09 X-TUID: jQOr7qvtE3Zg Hi Ludovic, Sorry I couldn't reply faster. 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. 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 an= d 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: > > $ 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 au= thenticate))' > % 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 =E2=80=98guix help=E2=80=99; the 2nd run +/- = emulates 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 t= o get > at that synopsis: > > scheme@(guile-user)> ,use(guix modules) > scheme@(guile-user)> (length (source-module-closure '((guix scripts syste= m) (guix scripts authenticate)))) > $10 =3D 439 > scheme@(guile-user)> (length (source-module-closure '((guix scripts) (gui= x ui)))) > $11 =3D 31 > > Memory usage would also be very different: > > $ \time guix help >/dev/null > 0.07user 0.01system 0:00.06elapsed 128%CPU (0avgtext+0avgdata 35348maxres= ident)k > 0inputs+0outputs (0major+3906minor)pagefaults 0swaps > $ \time guile -c '(use-modules (guix scripts system) (guix scripts authen= ticate))' > 0.42user 0.05system 0:00.37elapsed 128%CPU (0avgtext+0avgdata 166916maxre= sident)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=E2=80=99s a good way to not contribute to the general impressi= on 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 sub= jective. ;-) 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=E2=80=99ll send a v2! Thanks! I'm late, but LGTM, thank you. Maxim