unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
* [bug#45102] [PATCH 0/4] Making fewer 'stat' calls during startup
@ 2020-12-07 15:21 Ludovic Courtès
  2020-12-07 15:29 ` [bug#45102] [PATCH 1/4] build: 'script/guix' uses our own 'guile' executable Ludovic Courtès
  2020-12-08 13:50 ` [bug#45102] [PATCH 0/4] Making fewer 'stat' calls during startup Mathieu Othacehe
  0 siblings, 2 replies; 8+ messages in thread
From: Ludovic Courtès @ 2020-12-07 15:21 UTC (permalink / raw)
  To: 45102; +Cc: Ludovic Courtès

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





^ permalink raw reply	[flat|nested] 8+ messages in thread

* [bug#45102] [PATCH 1/4] build: 'script/guix' uses our own 'guile' executable.
  2020-12-07 15:21 [bug#45102] [PATCH 0/4] Making fewer 'stat' calls during startup Ludovic Courtès
@ 2020-12-07 15:29 ` Ludovic Courtès
  2020-12-07 15:29   ` [bug#45102] [PATCH 2/4] self: Move Guile early in the module search path Ludovic Courtès
                     ` (2 more replies)
  2020-12-08 13:50 ` [bug#45102] [PATCH 0/4] Making fewer 'stat' calls during startup Mathieu Othacehe
  1 sibling, 3 replies; 8+ messages in thread
From: Ludovic Courtès @ 2020-12-07 15:29 UTC (permalink / raw)
  To: 45102; +Cc: Ludovic Courtès

* 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





^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [bug#45102] [PATCH 2/4] self: Move Guile early in the module search path.
  2020-12-07 15:29 ` [bug#45102] [PATCH 1/4] build: 'script/guix' uses our own 'guile' executable Ludovic Courtès
@ 2020-12-07 15:29   ` Ludovic Courtès
  2020-12-07 15:29   ` [bug#45102] [PATCH 3/4] guix: 'guile' executable ignores GUILE_LOAD_PATH during startup Ludovic Courtès
  2020-12-07 15:29   ` [bug#45102] [PATCH 4/4] self: Remove the empty string from '%load-extensions' Ludovic Courtès
  2 siblings, 0 replies; 8+ messages in thread
From: Ludovic Courtès @ 2020-12-07 15:29 UTC (permalink / raw)
  To: 45102; +Cc: Ludovic Courtès

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





^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [bug#45102] [PATCH 3/4] guix: 'guile' executable ignores GUILE_LOAD_PATH during startup.
  2020-12-07 15:29 ` [bug#45102] [PATCH 1/4] build: 'script/guix' uses our own 'guile' executable Ludovic Courtès
  2020-12-07 15:29   ` [bug#45102] [PATCH 2/4] self: Move Guile early in the module search path Ludovic Courtès
@ 2020-12-07 15:29   ` Ludovic Courtès
  2020-12-07 15:29   ` [bug#45102] [PATCH 4/4] self: Remove the empty string from '%load-extensions' Ludovic Courtès
  2 siblings, 0 replies; 8+ messages in thread
From: Ludovic Courtès @ 2020-12-07 15:29 UTC (permalink / raw)
  To: 45102; +Cc: Ludovic Courtès

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@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





^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [bug#45102] [PATCH 4/4] self: Remove the empty string from '%load-extensions'.
  2020-12-07 15:29 ` [bug#45102] [PATCH 1/4] build: 'script/guix' uses our own 'guile' executable Ludovic Courtès
  2020-12-07 15:29   ` [bug#45102] [PATCH 2/4] self: Move Guile early in the module search path Ludovic Courtès
  2020-12-07 15:29   ` [bug#45102] [PATCH 3/4] guix: 'guile' executable ignores GUILE_LOAD_PATH during startup Ludovic Courtès
@ 2020-12-07 15:29   ` Ludovic Courtès
  2 siblings, 0 replies; 8+ messages in thread
From: Ludovic Courtès @ 2020-12-07 15:29 UTC (permalink / raw)
  To: 45102; +Cc: Ludovic Courtès

* 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





^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [bug#45102] [PATCH 0/4] Making fewer 'stat' calls during startup
  2020-12-07 15:21 [bug#45102] [PATCH 0/4] Making fewer 'stat' calls during startup Ludovic Courtès
  2020-12-07 15:29 ` [bug#45102] [PATCH 1/4] build: 'script/guix' uses our own 'guile' executable Ludovic Courtès
@ 2020-12-08 13:50 ` Mathieu Othacehe
  2020-12-08 20:24   ` Ludovic Courtès
  1 sibling, 1 reply; 8+ messages in thread
From: Mathieu Othacehe @ 2020-12-08 13:50 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 45102


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




^ permalink raw reply	[flat|nested] 8+ messages in thread

* [bug#45102] [PATCH 0/4] Making fewer 'stat' calls during startup
  2020-12-08 13:50 ` [bug#45102] [PATCH 0/4] Making fewer 'stat' calls during startup Mathieu Othacehe
@ 2020-12-08 20:24   ` Ludovic Courtès
  2020-12-11 18:12     ` bug#45102: " Ludovic Courtès
  0 siblings, 1 reply; 8+ messages in thread
From: Ludovic Courtès @ 2020-12-08 20:24 UTC (permalink / raw)
  To: Mathieu Othacehe; +Cc: 45102

Hi,

Mathieu Othacehe <othacehe@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’.




^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#45102: [PATCH 0/4] Making fewer 'stat' calls during startup
  2020-12-08 20:24   ` Ludovic Courtès
@ 2020-12-11 18:12     ` Ludovic Courtès
  0 siblings, 0 replies; 8+ messages in thread
From: Ludovic Courtès @ 2020-12-11 18:12 UTC (permalink / raw)
  To: Mathieu Othacehe; +Cc: 45102-done

Hi!

Pushed as 41d01b4e2e74a3e655bac03c241f0de7cb34b75f!

Ludo’.




^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2020-12-11 18:36 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-12-07 15:21 [bug#45102] [PATCH 0/4] Making fewer 'stat' calls during startup Ludovic Courtès
2020-12-07 15:29 ` [bug#45102] [PATCH 1/4] build: 'script/guix' uses our own 'guile' executable Ludovic Courtès
2020-12-07 15:29   ` [bug#45102] [PATCH 2/4] self: Move Guile early in the module search path Ludovic Courtès
2020-12-07 15:29   ` [bug#45102] [PATCH 3/4] guix: 'guile' executable ignores GUILE_LOAD_PATH during startup Ludovic Courtès
2020-12-07 15:29   ` [bug#45102] [PATCH 4/4] self: Remove the empty string from '%load-extensions' Ludovic Courtès
2020-12-08 13:50 ` [bug#45102] [PATCH 0/4] Making fewer 'stat' calls during startup Mathieu Othacehe
2020-12-08 20:24   ` Ludovic Courtès
2020-12-11 18:12     ` bug#45102: " Ludovic Courtès

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