all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* [bug#49919] [PATCH core-updates 0/2] build-system/go: Enable parallelism.
@ 2021-08-07  4:45 Sarah Morgensen
  2021-08-07  4:48 ` [bug#49919] [PATCH core-updates 1/2] build-system/go: Honor #:parallel-build? Sarah Morgensen
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Sarah Morgensen @ 2021-08-07  4:45 UTC (permalink / raw)
  To: 49919

Hello Guix,

These patches give the Go build system the standard parallelism levers. I would
have thought that Go was already detecting the correct number for GOMAXPROCS,
but after I made this same change for the bootstrapping Go compiler, Efraim
found that it cut the build time nearly in half [0].

[0] https://issues.guix.gnu.org/49616

--
Sarah Morgensen (2):
  build-system/go: Honor #:parallel-build?.
  build-system/go: Honor #:parallel-tests?.

 guix/build-system/go.scm       |  5 +++++
 guix/build/go-build-system.scm | 12 ++++++++++--
 2 files changed, 15 insertions(+), 2 deletions(-)


base-commit: 693d75e859150601145b7f7303f61d4f48e76927
-- 
2.31.1





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

* [bug#49919] [PATCH core-updates 1/2] build-system/go: Honor #:parallel-build?.
  2021-08-07  4:45 [bug#49919] [PATCH core-updates 0/2] build-system/go: Enable parallelism Sarah Morgensen
@ 2021-08-07  4:48 ` Sarah Morgensen
  2021-08-07  4:48 ` [bug#49919] [PATCH core-updates 2/2] build-system/go: Honor #:parallel-tests? Sarah Morgensen
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Sarah Morgensen @ 2021-08-07  4:48 UTC (permalink / raw)
  To: 49919

guix/build/go-build-system.scm (build): Honor #:parallel-build?.
guix/build-system/go.scm (go-build): Add PARALLEL-BUILD? parameter.
[builder]: Use it.
---
 guix/build-system/go.scm       | 3 +++
 guix/build/go-build-system.scm | 7 ++++++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/guix/build-system/go.scm b/guix/build-system/go.scm
index 100d1db4b6..a5aa21b99e 100644
--- a/guix/build-system/go.scm
+++ b/guix/build-system/go.scm
@@ -3,6 +3,7 @@
 ;;; Copyright © 2017 Leo Famulari <leo@famulari.name>
 ;;; Copyright © 2020 Jakub Kądziołka <kuba@kadziolka.net>
 ;;; Copyright © 2021 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -129,6 +130,7 @@ commit hash and its date rather than a proper release tag."
                    (unpack-path "")
                    (build-flags ''())
                    (tests? #t)
+                   (parallel-build? #t)
                    (allow-go-reference? #f)
                    (system (%current-system))
                    (guile #f)
@@ -153,6 +155,7 @@ commit hash and its date rather than a proper release tag."
                     #:unpack-path #$unpack-path
                     #:build-flags #$build-flags
                     #:tests? #$tests?
+                    #:parallel-build? #$parallel-build?
                     #:allow-go-reference? #$allow-go-reference?
                     #:inputs #$(input-tuples->gexp inputs)))))
 
diff --git a/guix/build/go-build-system.scm b/guix/build/go-build-system.scm
index 227df820db..521bad9667 100644
--- a/guix/build/go-build-system.scm
+++ b/guix/build/go-build-system.scm
@@ -5,6 +5,7 @@
 ;;; Copyright © 2020 Jack Hill <jackhill@jackhill.us>
 ;;; Copyright © 2020 Jakub Kądziołka <kuba@kadziolka.net>
 ;;; Copyright © 2020 Efraim Flashner <efraim@flashner.co.il>
+;;; Copyright © 2021 Sarah Morgensen <iskarian@mgsn.dev>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -216,8 +217,12 @@ unpacking."
                 (_ #f))
               inputs))))
 
-(define* (build #:key import-path build-flags #:allow-other-keys)
+(define* (build #:key import-path build-flags (parallel-build? #t)
+                #:allow-other-keys)
   "Build the package named by IMPORT-PATH."
+  (let* ((njobs (if parallel-build? (parallel-job-count) 1)))
+    (setenv "GOMAXPROCS" (number->string njobs)))
+
   (with-throw-handler
     #t
     (lambda _
-- 
2.31.1





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

* [bug#49919] [PATCH core-updates 2/2] build-system/go: Honor #:parallel-tests?.
  2021-08-07  4:45 [bug#49919] [PATCH core-updates 0/2] build-system/go: Enable parallelism Sarah Morgensen
  2021-08-07  4:48 ` [bug#49919] [PATCH core-updates 1/2] build-system/go: Honor #:parallel-build? Sarah Morgensen
@ 2021-08-07  4:48 ` Sarah Morgensen
  2021-08-19 15:13 ` [bug#49919] [PATCH core-updates 0/2] build-system/go: Enable parallelism Mathieu Othacehe
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Sarah Morgensen @ 2021-08-07  4:48 UTC (permalink / raw)
  To: 49919

guix/build/go-build-system.scm (build): Honor #:parallel-tests?.
guix/build-system/go.scm (go-build): Add PARALLEL-TESTS? parameter.
[builder]: Use it.
---
 guix/build-system/go.scm       | 2 ++
 guix/build/go-build-system.scm | 5 ++++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/guix/build-system/go.scm b/guix/build-system/go.scm
index a5aa21b99e..7b66163779 100644
--- a/guix/build-system/go.scm
+++ b/guix/build-system/go.scm
@@ -131,6 +131,7 @@ commit hash and its date rather than a proper release tag."
                    (build-flags ''())
                    (tests? #t)
                    (parallel-build? #t)
+                   (parallel-tests? #t)
                    (allow-go-reference? #f)
                    (system (%current-system))
                    (guile #f)
@@ -156,6 +157,7 @@ commit hash and its date rather than a proper release tag."
                     #:build-flags #$build-flags
                     #:tests? #$tests?
                     #:parallel-build? #$parallel-build?
+                    #:parallel-tests? #$parallel-tests?
                     #:allow-go-reference? #$allow-go-reference?
                     #:inputs #$(input-tuples->gexp inputs)))))
 
diff --git a/guix/build/go-build-system.scm b/guix/build/go-build-system.scm
index 521bad9667..0ad580a484 100644
--- a/guix/build/go-build-system.scm
+++ b/guix/build/go-build-system.scm
@@ -239,9 +239,12 @@ unpacking."
       (invoke "go" "env"))))
 
 ;; Can this also install commands???
-(define* (check #:key tests? import-path #:allow-other-keys)
+(define* (check #:key tests? import-path (parallel-tests? #t)
+                #:allow-other-keys)
   "Run the tests for the package named by IMPORT-PATH."
   (when tests?
+    (let* ((njobs (if parallel-tests? (parallel-job-count) 1)))
+      (setenv "GOMAXPROCS" (number->string njobs)))
     (invoke "go" "test" import-path))
   #t)
 
-- 
2.31.1





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

* [bug#49919] [PATCH core-updates 0/2] build-system/go: Enable parallelism.
  2021-08-07  4:45 [bug#49919] [PATCH core-updates 0/2] build-system/go: Enable parallelism Sarah Morgensen
  2021-08-07  4:48 ` [bug#49919] [PATCH core-updates 1/2] build-system/go: Honor #:parallel-build? Sarah Morgensen
  2021-08-07  4:48 ` [bug#49919] [PATCH core-updates 2/2] build-system/go: Honor #:parallel-tests? Sarah Morgensen
@ 2021-08-19 15:13 ` Mathieu Othacehe
  2021-12-15 10:35   ` Efraim Flashner
  2021-08-31 10:03 ` [bug#49919] " zimoun
  2021-08-31 16:06 ` Sarah Morgensen
  4 siblings, 1 reply; 9+ messages in thread
From: Mathieu Othacehe @ 2021-08-19 15:13 UTC (permalink / raw)
  To: Sarah Morgensen; +Cc: 49919


Hello Sarah,

> These patches give the Go build system the standard parallelism levers. I would
> have thought that Go was already detecting the correct number for GOMAXPROCS,
> but after I made this same change for the bootstrapping Go compiler, Efraim
> found that it cut the build time nearly in half [0].

This looks good, but I'd rather have Cuirass fixed before pushing this
series so that we can evaluate the impact on all Go packages and
potentially find some regressions.

Thanks,

Mathieu




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

* [bug#49919] [PATCH core-updates 0/2] build-system/go: Enable parallelism.
  2021-08-07  4:45 [bug#49919] [PATCH core-updates 0/2] build-system/go: Enable parallelism Sarah Morgensen
                   ` (2 preceding siblings ...)
  2021-08-19 15:13 ` [bug#49919] [PATCH core-updates 0/2] build-system/go: Enable parallelism Mathieu Othacehe
@ 2021-08-31 10:03 ` zimoun
  2021-08-31 16:06 ` Sarah Morgensen
  4 siblings, 0 replies; 9+ messages in thread
From: zimoun @ 2021-08-31 10:03 UTC (permalink / raw)
  To: Sarah Morgensen, 49919

Hi,

On Fri, 06 Aug 2021 at 21:45, Sarah Morgensen <iskarian@mgsn.dev> wrote:

> These patches give the Go build system the standard parallelism levers. I would
> have thought that Go was already detecting the correct number for GOMAXPROCS,
> but after I made this same change for the bootstrapping Go compiler, Efraim
> found that it cut the build time nearly in half [0].
>
> [0] https://issues.guix.gnu.org/49616

Honoring the parallelism, do the Go packages still build
deterministically?  If not, the default should be still non parallel, as
it is for Haskell packages, IIRC.  However, maybe there is room for a
package transformation ’--with-parallelism’ to easily turn on when speed
matters more than reproducibility.

All the best,
simon




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

* [bug#49919] [PATCH core-updates 0/2] build-system/go: Enable parallelism.
  2021-08-07  4:45 [bug#49919] [PATCH core-updates 0/2] build-system/go: Enable parallelism Sarah Morgensen
                   ` (3 preceding siblings ...)
  2021-08-31 10:03 ` [bug#49919] " zimoun
@ 2021-08-31 16:06 ` Sarah Morgensen
  4 siblings, 0 replies; 9+ messages in thread
From: Sarah Morgensen @ 2021-08-31 16:06 UTC (permalink / raw)
  To: zimoun; +Cc: 49919

Hello,

zimoun <zimon.toutoune@gmail.com> writes:

> Hi,
>
> On Fri, 06 Aug 2021 at 21:45, Sarah Morgensen <iskarian@mgsn.dev> wrote:
>
>> These patches give the Go build system the standard parallelism levers. I would
>> have thought that Go was already detecting the correct number for GOMAXPROCS,
>> but after I made this same change for the bootstrapping Go compiler, Efraim
>> found that it cut the build time nearly in half [0].
>>
>> [0] https://issues.guix.gnu.org/49616
>
> Honoring the parallelism, do the Go packages still build
> deterministically?  If not, the default should be still non parallel, as
> it is for Haskell packages, IIRC.  However, maybe there is room for a
> package transformation ’--with-parallelism’ to easily turn on when speed
> matters more than reproducibility.

I haven't personally tested this aside from a few casual runs with
--check, but from everything I understand about Go, parallelism does not
affect build determinism.  Each build unit should still be compiled by a
single worker sequentially, but parallelism enables separate build units
to be compiled in parallel.

--
Sarah




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

* [bug#49919] [PATCH core-updates 0/2] build-system/go: Enable parallelism.
  2021-08-19 15:13 ` [bug#49919] [PATCH core-updates 0/2] build-system/go: Enable parallelism Mathieu Othacehe
@ 2021-12-15 10:35   ` Efraim Flashner
  2021-12-15 10:43     ` Mathieu Othacehe
  0 siblings, 1 reply; 9+ messages in thread
From: Efraim Flashner @ 2021-12-15 10:35 UTC (permalink / raw)
  To: Mathieu Othacehe; +Cc: Sarah Morgensen, 49919

[-- Attachment #1: Type: text/plain, Size: 875 bytes --]

On Thu, Aug 19, 2021 at 05:13:52PM +0200, Mathieu Othacehe wrote:
> 
> Hello Sarah,
> 
> > These patches give the Go build system the standard parallelism levers. I would
> > have thought that Go was already detecting the correct number for GOMAXPROCS,
> > but after I made this same change for the bootstrapping Go compiler, Efraim
> > found that it cut the build time nearly in half [0].
> 
> This looks good, but I'd rather have Cuirass fixed before pushing this
> series so that we can evaluate the impact on all Go packages and
> potentially find some regressions.
> 

Friendly ping to see if Cuirass is ready for these patches to be pushed.

-- 
Efraim Flashner   <efraim@flashner.co.il>   רנשלפ םירפא
GPG key = A28B F40C 3E55 1372 662D  14F7 41AA E7DC CA3D 8351
Confidentiality cannot be guaranteed on emails sent or received unencrypted

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* [bug#49919] [PATCH core-updates 0/2] build-system/go: Enable parallelism.
  2021-12-15 10:35   ` Efraim Flashner
@ 2021-12-15 10:43     ` Mathieu Othacehe
  2024-01-20 20:49       ` bug#49919: " Maxim Cournoyer
  0 siblings, 1 reply; 9+ messages in thread
From: Mathieu Othacehe @ 2021-12-15 10:43 UTC (permalink / raw)
  To: Efraim Flashner; +Cc: Sarah Morgensen, 49919


Hey,

> Friendly ping to see if Cuirass is ready for these patches to be pushed.

Not really, I need to reconfigure Berlin and I'm hitting multiple
issues, such as a php build failure :(.

Mathieu




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

* bug#49919: [PATCH core-updates 0/2] build-system/go: Enable parallelism.
  2021-12-15 10:43     ` Mathieu Othacehe
@ 2024-01-20 20:49       ` Maxim Cournoyer
  0 siblings, 0 replies; 9+ messages in thread
From: Maxim Cournoyer @ 2024-01-20 20:49 UTC (permalink / raw)
  To: Mathieu Othacehe; +Cc: 49919-done, Sarah Morgensen, Efraim Flashner

Hello,

Mathieu Othacehe <othacehe@gnu.org> writes:

> Hey,
>
>> Friendly ping to see if Cuirass is ready for these patches to be pushed.
>
> Not really, I need to reconfigure Berlin and I'm hitting multiple
> issues, such as a php build failure :(.

Queued for applying locally, at last.  Closing.

-- 
Thanks,
Maxim




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

end of thread, other threads:[~2024-01-20 20:51 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-08-07  4:45 [bug#49919] [PATCH core-updates 0/2] build-system/go: Enable parallelism Sarah Morgensen
2021-08-07  4:48 ` [bug#49919] [PATCH core-updates 1/2] build-system/go: Honor #:parallel-build? Sarah Morgensen
2021-08-07  4:48 ` [bug#49919] [PATCH core-updates 2/2] build-system/go: Honor #:parallel-tests? Sarah Morgensen
2021-08-19 15:13 ` [bug#49919] [PATCH core-updates 0/2] build-system/go: Enable parallelism Mathieu Othacehe
2021-12-15 10:35   ` Efraim Flashner
2021-12-15 10:43     ` Mathieu Othacehe
2024-01-20 20:49       ` bug#49919: " Maxim Cournoyer
2021-08-31 10:03 ` [bug#49919] " zimoun
2021-08-31 16:06 ` Sarah Morgensen

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/guix.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.