GNU bug report logs - #45102
[PATCH 0/4] Making fewer 'stat' calls during startup

Previous Next

Package: guix-patches;

Reported by: Ludovic Courtès <ludo <at> gnu.org>

Date: Mon, 7 Dec 2020 15:23:01 UTC

Severity: normal

Tags: patch

Done: Ludovic Courtès <ludo <at> gnu.org>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 45102 in the body.
You can then email your comments to 45102 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to guix-patches <at> gnu.org:
bug#45102; Package guix-patches. (Mon, 07 Dec 2020 15:23:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to Ludovic Courtès <ludo <at> gnu.org>:
New bug report received and forwarded. Copy sent to guix-patches <at> gnu.org. (Mon, 07 Dec 2020 15:23:01 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: guix-patches <at> gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>
Subject: [PATCH 0/4] Making fewer 'stat' calls during startup
Date: Mon,  7 Dec 2020 16:21:59 +0100
Hi Guix!

On Guix System, with 6 entries in $GUILE_LOAD_PATH (which should just
be 2 entries, one for /run/current-system and one for ~/.guix-profile),
I see this:

--8<---------------cut here---------------start------------->8---
$ strace -c -e stat,openat guix
guix: mankas komanda nomo
Provu 'guix --help' por pli da informo.
% time     seconds  usecs/call     calls    errors syscall
------ ----------- ----------- --------- --------- ----------------
 90.94    0.002207           0      2283      2057 stat
  9.06    0.000220           0       334       177 openat
------ ----------- ----------- --------- --------- ----------------
100.00    0.002427           0      2617      2234 total
--8<---------------cut here---------------end--------------->8---

This is a lot of ‘stat’ calls for nothing, 4 times more than when
GUILE_LOAD_PATH is unset:

--8<---------------cut here---------------start------------->8---
$ env -i $(type -P strace) -c -e stat,openat $(type -P guix)
guix: missing command name
Try `guix --help' for more information.
% time     seconds  usecs/call     calls    errors syscall
------ ----------- ----------- --------- --------- ----------------
 57.89    0.000187           0       495       272 stat
 42.11    0.000136           0       321       175 openat
------ ----------- ----------- --------- --------- ----------------
100.00    0.000323           0       816       447 total
--8<---------------cut here---------------end--------------->8---

This patch series takes a sledgehammer approach to always have as
few ‘stat’ calls as possible during startup.  After the change, I get:

--8<---------------cut here---------------start------------->8---
$ strace -e stat,openat -c /gnu/store/g9gylj723si8i2cp8ia57a7kr4i8b1m9-guix-20201207.14/bin/guix 
guix: mankas komanda nomo
Provu 'guix --help' por pli da informo.
% time     seconds  usecs/call     calls    errors syscall
------ ----------- ----------- --------- --------- ----------------
 60.53    0.000322           0       454       231 stat
 39.47    0.000210           0       334       177 openat
------ ----------- ----------- --------- --------- ----------------
100.00    0.000532           0       788       408 total
$ env -i $(type -P strace) -e stat,openat -c /gnu/store/g9gylj723si8i2cp8ia57a7kr4i8b1m9-guix-20201207.14/bin/guix 
guix: missing command name
Try `guix --help' for more information.
% time     seconds  usecs/call     calls    errors syscall
------ ----------- ----------- --------- --------- ----------------
 69.92    0.000272           0       442       219 stat
 30.08    0.000117           0       321       175 openat
------ ----------- ----------- --------- --------- ----------------
100.00    0.000389           0       763       394 total
--8<---------------cut here---------------end--------------->8---

What impact does it have on startup time?  Hard to tell.
On my 4-year old x86_64 laptop with an SSD, it seems to be negligible,
even on a cold cache.  I suspect things are different on spinning
disks and on NFS.

The semantic difference should be invisible to users: their modules
are still visible and usable in manifests, in ‘guix repl’, etc.; they
just cannot take precedence over modules from Guile and from the
channels.  For instance, one cannot define a (gnu packages base)
module or (ice-9 rdelim), drop it in $GUILE_LOAD_PATH, and have that
module picked up in lieu of the original one.  I think that’s a
reasonable tradeoff.

Thoughts?

Ludo’.

Ludovic Courtès (4):
  build: 'script/guix' uses our own 'guile' executable.
  self: Move Guile early in the module search path.
  guix: 'guile' executable ignores GUILE_LOAD_PATH during startup.
  self: Remove the empty string from '%load-extensions'.

 Makefile.am                             |  1 +
 gnu/packages/aux-files/guile-launcher.c | 46 +++++++++++++++++++++++--
 guix/self.scm                           | 26 +++++++++-----
 scripts/guix.in                         |  2 +-
 4 files changed, 63 insertions(+), 12 deletions(-)

-- 
2.29.2





Information forwarded to guix-patches <at> gnu.org:
bug#45102; Package guix-patches. (Mon, 07 Dec 2020 15:31:02 GMT) Full text and rfc822 format available.

Message #8 received at 45102 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 45102 <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>
Subject: [PATCH 1/4] build: 'script/guix' uses our own 'guile' executable.
Date: Mon,  7 Dec 2020 16:29:56 +0100
* Makefile.am (do_subst): Substitute @abs_top_builddir@.
* scripts/guix.in: Use it.
---
 Makefile.am     | 1 +
 scripts/guix.in | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/Makefile.am b/Makefile.am
index 1a3ca227a4..9803ba5dc7 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -39,6 +39,7 @@ do_subst = $(SED)					\
   -e 's,[@]GUILE[@],$(GUILE),g'				\
   -e 's,[@]guilemoduledir[@],$(guilemoduledir),g'	\
   -e 's,[@]guileobjectdir[@],$(guileobjectdir),g'	\
+  -e 's,[@]abs_top_builddir[@],$(abs_top_builddir),g'	\
   -e 's,[@]localedir[@],$(localedir),g'
 
 scripts/guix: scripts/guix.in Makefile
diff --git a/scripts/guix.in b/scripts/guix.in
index 0a3ab1f64d..e0194d6ea2 100644
--- a/scripts/guix.in
+++ b/scripts/guix.in
@@ -1,4 +1,4 @@
-#!@GUILE@ \
+#!@abs_top_builddir@/guile \
 --no-auto-compile -e main -s
 !#
 ;;; GNU Guix --- Functional package management for GNU
-- 
2.29.2





Information forwarded to guix-patches <at> gnu.org:
bug#45102; Package guix-patches. (Mon, 07 Dec 2020 15:31:02 GMT) Full text and rfc822 format available.

Message #11 received at 45102 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 45102 <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>
Subject: [PATCH 2/4] self: Move Guile early in the module search path.
Date: Mon,  7 Dec 2020 16:29:57 +0100
Until now, Guile modules would first be searched for in
MODULE-DIRECTORY, then in each $GUILE_LOAD_PATH entry, and finally in
Guile itself.

* guix/self.scm (guix-command): Make GUILE the second entry in the
%LOAD-PATH and %LOAD-COMPILED-PATH.
---
 guix/self.scm | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/guix/self.scm b/guix/self.scm
index c0de14b79a..ca67f653fa 100644
--- a/guix/self.scm
+++ b/guix/self.scm
@@ -650,17 +650,22 @@ load path."
   (program-file "guix-command"
                 #~(begin
                     (set! %load-path
-                      (cons (string-append #$module-directory
-                                           "/share/guile/site/"
-                                           (effective-version))
-                            %load-path))
+                      (append (list (string-append #$module-directory
+                                                   "/share/guile/site/"
+                                                   (effective-version))
+                                    (string-append #$guile "/share/guile/"
+                                                   (effective-version)))
+                              %load-path))
 
                     (set! %load-compiled-path
-                      (cons (string-append #$module-directory
-                                           "/lib/guile/"
-                                           (effective-version)
-                                           "/site-ccache")
-                            %load-compiled-path))
+                      (append (list (string-append #$module-directory
+                                                   "/lib/guile/"
+                                                   (effective-version)
+                                                   "/site-ccache")
+                                    (string-append #$guile "/lib/guile/"
+                                                   (effective-version)
+                                                   "/ccache"))
+                              %load-compiled-path))
 
                     ;; To maximize the chances that locales are set up right
                     ;; out-of-the-box, bundle "common" UTF-8 locales.
-- 
2.29.2





Information forwarded to guix-patches <at> gnu.org:
bug#45102; Package guix-patches. (Mon, 07 Dec 2020 15:31:02 GMT) Full text and rfc822 format available.

Message #14 received at 45102 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 45102 <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>
Subject: [PATCH 3/4] guix: 'guile' executable ignores GUILE_LOAD_PATH during
 startup.
Date: Mon,  7 Dec 2020 16:29:58 +0100
When starting the 'guix' command, this ensures Guile modules are
immediately found instead of being search for in other directories.
This reduces the number of 'stat' calls during startup when
GUILE_LOAD_PATH is set to (almost) that of "env -i $(type -P guix)".

* gnu/packages/aux-files/guile-launcher.c (load_path)
(load_compiled_path): New variables.
(inner_main): Restore GUILE_LOAD_PATH and GUILE_LOAD_COMPILED_PATH and
set %load-path and %load-compiled-path accordingly.
(main): Save GUILE_LOAD_PATH and GUILE_LOAD_COMPILED_PATH and unset them.
---
 gnu/packages/aux-files/guile-launcher.c | 46 +++++++++++++++++++++++--
 1 file changed, 44 insertions(+), 2 deletions(-)

diff --git a/gnu/packages/aux-files/guile-launcher.c b/gnu/packages/aux-files/guile-launcher.c
index 886ede2846..1dd5d77e66 100644
--- a/gnu/packages/aux-files/guile-launcher.c
+++ b/gnu/packages/aux-files/guile-launcher.c
@@ -1,5 +1,5 @@
 /* GNU Guix --- Functional package management for GNU
-   Copyright 1996-1997,2000-2001,2006,2008,2011,2013,2018
+   Copyright 1996-1997,2000-2001,2006,2008,2011,2013,2018,2020
       Free Software Foundation, Inc.
    Copyright (C) 2020 Ludovic Courtès <ludo <at> gnu.org>
 
@@ -19,14 +19,47 @@
    along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.  */
 
 /* This file implements a variant of the 'guile' executable that does not
-   complain about locale issues.  */
+   complain about locale issues and arranges to reduce startup time by
+   ignoring GUILE_LOAD_PATH and GUILE_LOAD_COMPILED_PATH until it has
+   booted.  */
 
+#include <stdlib.h>
+#include <string.h>
 #include <locale.h>
 #include <libguile.h>
 
+/* Saved values of GUILE_LOAD_PATH and GUILE_LOAD_COMPILED_PATH.  */
+static const char *load_path, *load_compiled_path;
+
 static void
 inner_main (void *unused, int argc, char **argv)
 {
+  if (load_path != NULL)
+    {
+      setenv ("GUILE_LOAD_PATH", load_path, 1);
+      SCM load_path_var =
+	scm_c_public_lookup ("guile", "%load-path");
+      SCM addition =
+	scm_parse_path (scm_from_locale_string (load_path), SCM_EOL);
+      scm_variable_set_x (load_path_var,
+			  scm_append
+			  (scm_list_2 (scm_variable_ref (load_path_var),
+				       addition)));
+    }
+
+  if (load_compiled_path != NULL)
+    {
+      setenv ("GUILE_LOAD_COMPILED_PATH", load_compiled_path, 1);
+      SCM load_compiled_path_var =
+	scm_c_public_lookup ("guile", "%load-compiled-path");
+      SCM addition =
+	scm_parse_path (scm_from_locale_string (load_compiled_path), SCM_EOL);
+      scm_variable_set_x (load_compiled_path_var,
+			  scm_append
+			  (scm_list_2 (scm_variable_ref (load_compiled_path_var),
+				       addition)));
+    }
+
   scm_shell (argc, argv);
 }
 
@@ -40,6 +73,15 @@ main (int argc, char **argv)
        which is always preferable over the C locale.  */
     setlocale (LC_ALL, "en_US.utf8");
 
+  const char *str;
+  str = getenv ("GUILE_LOAD_PATH");
+  load_path = str != NULL ? strdup (str) : NULL;
+  str = getenv ("GUILE_LOAD_COMPILED_PATH");
+  load_compiled_path = str ? strdup (str) : NULL;
+
+  unsetenv ("GUILE_LOAD_PATH");
+  unsetenv ("GUILE_LOAD_COMPILED_PATH");
+
   scm_install_gmp_memory_functions = 1;
   scm_boot_guile (argc, argv, inner_main, 0);
   return 0; /* never reached */
-- 
2.29.2





Information forwarded to guix-patches <at> gnu.org:
bug#45102; Package guix-patches. (Mon, 07 Dec 2020 15:31:03 GMT) Full text and rfc822 format available.

Message #17 received at 45102 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 45102 <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>
Subject: [PATCH 4/4] self: Remove the empty string from '%load-extensions'.
Date: Mon,  7 Dec 2020 16:29:59 +0100
* guix/self.scm (guix-command): Set '%load-extensions'.
---
 guix/self.scm | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/guix/self.scm b/guix/self.scm
index ca67f653fa..7cda6656c9 100644
--- a/guix/self.scm
+++ b/guix/self.scm
@@ -649,6 +649,9 @@ load path."
 
   (program-file "guix-command"
                 #~(begin
+                    ;; Remove the empty extension from the search path.
+                    (set! %load-extensions '(".scm"))
+
                     (set! %load-path
                       (append (list (string-append #$module-directory
                                                    "/share/guile/site/"
-- 
2.29.2





Information forwarded to guix-patches <at> gnu.org:
bug#45102; Package guix-patches. (Tue, 08 Dec 2020 13:51:02 GMT) Full text and rfc822 format available.

Message #20 received at 45102 <at> debbugs.gnu.org (full text, mbox):

From: Mathieu Othacehe <othacehe <at> gnu.org>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 45102 <at> debbugs.gnu.org
Subject: Re: [bug#45102] [PATCH 0/4] Making fewer 'stat' calls during startup
Date: Tue, 08 Dec 2020 14:50:08 +0100
Hey Ludo,

> The semantic difference should be invisible to users: their modules
> are still visible and usable in manifests, in ‘guix repl’, etc.; they
> just cannot take precedence over modules from Guile and from the
> channels.  For instance, one cannot define a (gnu packages base)
> module or (ice-9 rdelim), drop it in $GUILE_LOAD_PATH, and have that
> module picked up in lieu of the original one.  I think that’s a
> reasonable tradeoff.

I think that's reasonable too. I tested it locally and have the
following command:

--8<---------------cut here---------------start------------->8---
strace -c -e stat,openat ./pre-inst-env guix
--8<---------------cut here---------------end--------------->8---

drop from 1671 calls to 1017 calls, which is nice.

The patchset looks good to me.

Thanks,

Mathieu




Information forwarded to guix-patches <at> gnu.org:
bug#45102; Package guix-patches. (Tue, 08 Dec 2020 20:25:02 GMT) Full text and rfc822 format available.

Message #23 received at 45102 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Mathieu Othacehe <othacehe <at> gnu.org>
Cc: 45102 <at> debbugs.gnu.org
Subject: Re: [bug#45102] [PATCH 0/4] Making fewer 'stat' calls during startup
Date: Tue, 08 Dec 2020 21:24:29 +0100
Hi,

Mathieu Othacehe <othacehe <at> gnu.org> skribis:

>> The semantic difference should be invisible to users: their modules
>> are still visible and usable in manifests, in ‘guix repl’, etc.; they
>> just cannot take precedence over modules from Guile and from the
>> channels.  For instance, one cannot define a (gnu packages base)
>> module or (ice-9 rdelim), drop it in $GUILE_LOAD_PATH, and have that
>> module picked up in lieu of the original one.  I think that’s a
>> reasonable tradeoff.
>
> I think that's reasonable too. I tested it locally and have the
> following command:
>
> strace -c -e stat,openat ./pre-inst-env guix
>
> drop from 1671 calls to 1017 calls, which is nice.

You should try:

  ./pre-inst-env strace -c stat guix

but even then you’ll get more ‘stat’ calls than the ‘guix’ command
provided by ‘guix pull’.

You can run, say:

  strace -c $(make as-derivation)/bin/guix

> The patchset looks good to me.

Cool, thanks!

Ludo’.




Reply sent to Ludovic Courtès <ludo <at> gnu.org>:
You have taken responsibility. (Fri, 11 Dec 2020 18:13:01 GMT) Full text and rfc822 format available.

Notification sent to Ludovic Courtès <ludo <at> gnu.org>:
bug acknowledged by developer. (Fri, 11 Dec 2020 18:13:02 GMT) Full text and rfc822 format available.

Message #28 received at 45102-done <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Mathieu Othacehe <othacehe <at> gnu.org>
Cc: 45102-done <at> debbugs.gnu.org
Subject: Re: [bug#45102] [PATCH 0/4] Making fewer 'stat' calls during startup
Date: Fri, 11 Dec 2020 19:12:21 +0100
Hi!

Pushed as 41d01b4e2e74a3e655bac03c241f0de7cb34b75f!

Ludo’.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Sat, 09 Jan 2021 12:24:03 GMT) Full text and rfc822 format available.

This bug report was last modified 4 years and 161 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.