unofficial mirror of guix-devel@gnu.org 
 help / color / mirror / Atom feed
* Test parallelism with CMake
@ 2021-10-09  7:56 Liliana Marie Prikler
  2021-10-12 17:28 ` Greg Hogan
  2021-10-22 13:10 ` Greg Hogan
  0 siblings, 2 replies; 9+ messages in thread
From: Liliana Marie Prikler @ 2021-10-09  7:56 UTC (permalink / raw)
  To: code, guix-devel

Hi Greg,

for the purposes of GNU Guix, #:parallel-build? and #:parallel-tests?
are distinct flags and the latter (if implemented) would apply to the
check phase.  Our cmake-build-system in this case defers to gnu-build-
system, which ought to insert -jN as you described.

It could be that the package(s) you build have disabled parallel tests
for some reason (breakage, reproducibility, ...).  If not there would
be issues passing those arguments, which I don't believe the case from
static analysis however.

Cheers,
Liliana



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

* Re: Test parallelism with CMake
  2021-10-09  7:56 Test parallelism with CMake Liliana Marie Prikler
@ 2021-10-12 17:28 ` Greg Hogan
  2021-10-22 13:10 ` Greg Hogan
  1 sibling, 0 replies; 9+ messages in thread
From: Greg Hogan @ 2021-10-12 17:28 UTC (permalink / raw)
  To: Liliana Marie Prikler; +Cc: guix-devel

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

Hi Liliana,

The packages I am building do not disable parallel tests.

Greg

On Sat, Oct 9, 2021 at 3:56 AM Liliana Marie Prikler <
liliana.prikler@gmail.com> wrote:

> Hi Greg,
>
> for the purposes of GNU Guix, #:parallel-build? and #:parallel-tests?
> are distinct flags and the latter (if implemented) would apply to the
> check phase.  Our cmake-build-system in this case defers to gnu-build-
> system, which ought to insert -jN as you described.
>
> It could be that the package(s) you build have disabled parallel tests
> for some reason (breakage, reproducibility, ...).  If not there would
> be issues passing those arguments, which I don't believe the case from
> static analysis however.
>
> Cheers,
> Liliana
>
>

[-- Attachment #2: Type: text/html, Size: 1152 bytes --]

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

* Re: Test parallelism with CMake
  2021-10-09  7:56 Test parallelism with CMake Liliana Marie Prikler
  2021-10-12 17:28 ` Greg Hogan
@ 2021-10-22 13:10 ` Greg Hogan
  2021-10-22 18:28   ` Liliana Marie Prikler
  2021-10-29 12:09   ` Ludovic Courtès
  1 sibling, 2 replies; 9+ messages in thread
From: Greg Hogan @ 2021-10-22 13:10 UTC (permalink / raw)
  To: Liliana Marie Prikler, Ludovic Courtès, guix-devel

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

On Sat, Oct 9, 2021 at 3:56 AM Liliana Marie Prikler <
liliana.prikler@gmail.com> wrote:

> for the purposes of GNU Guix, #:parallel-build? and #:parallel-tests?
> are distinct flags and the latter (if implemented) would apply to the
> check phase.  Our cmake-build-system in this case defers to gnu-build-
> system, which ought to insert -jN as you described.
>

On Thu, Oct 21, 2021 at 3:30 PM Ludovic Courtès <ludo@gnu.org> wrote:

> Does CMake generate makefiles targets that would allow tests to run in
> parallel?  How does that even work in CMake?
>

Hi Liliana and Ludo’,

The cmake-build-system does defer to gnu-build-system, which calls `make
test -jN`; however, CMake generated Makefile specifies 'test' as a single
target (the Ninja generator suffers from the same issue) so `ctest` is run
without parallelism.

To run CMake tests with parallelism the cmake-build-system should directly
call `ctest` with the configured parallelism (same '-jN' argument). The
cmake-build-system's check method is essentially untouched from March, 2013
(commit c6bded8a) so this issue has existed from the beginning.

I made an inelegant patch to my local guix repo, essentially replacing
cmake-build-system:check with a copy of gnu-build-system:check and changing
'apply invoke "make" test-target' to 'apply invoke "ctest"'. This works,
although the package I was working on requires parallel tests to be
disabled. Once the expectation is set that code will be run serially it is
difficult to change the default to parallel execution.

It seems that we should at a minimum document the issue in
cmake-build-system:check. We could patch cmake-build-system to enable test
parallelism and explicitly disable that setting for packages which succeed
before but fail after making the change. What do you think?

Greg

[-- Attachment #2: Type: text/html, Size: 2538 bytes --]

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

* Re: Test parallelism with CMake
  2021-10-22 13:10 ` Greg Hogan
@ 2021-10-22 18:28   ` Liliana Marie Prikler
  2021-10-29 12:09   ` Ludovic Courtès
  1 sibling, 0 replies; 9+ messages in thread
From: Liliana Marie Prikler @ 2021-10-22 18:28 UTC (permalink / raw)
  To: Greg Hogan, Ludovic Courtès, guix-devel

Hi,

Am Freitag, den 22.10.2021, 09:10 -0400 schrieb Greg Hogan:
> The cmake-build-system does defer to gnu-build-system, which calls
> `make test -jN`; however, CMake generated Makefile specifies 'test'
> as a single target (the Ninja generator suffers from the same issue)
> so `ctest` is run without parallelism.
That does sound like a CMake bug.  Has no one on their end addressed
that like since the inception of CMake?

> To run CMake tests with parallelism the cmake-build-system should
> directly call `ctest` with the configured parallelism (same '-jN'
> argument). The cmake-build-system's check method is essentially
> untouched from March, 2013 (commit c6bded8a) so this issue has
> existed from the beginning.
> 
> I made an inelegant patch to my local guix repo, essentially
> replacing cmake-build-system:check with a copy of gnu-build-
> system:check and changing 'apply invoke "make" test-target' to 'apply
> invoke "ctest"'. This works, although the package I was working on
> requires parallel tests to be disabled. Once the expectation is set
> that code will be run serially it is difficult to change the default
> to parallel execution.
> 
> It seems that we should at a minimum document the issue in cmake-
> build-system:check. We could patch cmake-build-system to enable test
> parallelism and explicitly disable that setting for packages which
> succeed before but fail after making the change. What do you think?
I'm not too sure on any of this.  For one, we'd have to survey whether
really all CMake-based packages use ctest to build and not any other
tool of their choosing.  Next, we'd have to adjust the calling
conventions to add support for ctest command syntax, given that it is
indeed another tool with probably conventions from make itself. 
Finally, messing with cmake-build-system would most likely result in a
world rebuild, so we can realistically do this on core-updates.

Regards,
Liliana



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

* Re: Test parallelism with CMake
  2021-10-22 13:10 ` Greg Hogan
  2021-10-22 18:28   ` Liliana Marie Prikler
@ 2021-10-29 12:09   ` Ludovic Courtès
  2021-11-03 14:47     ` Greg Hogan
  1 sibling, 1 reply; 9+ messages in thread
From: Ludovic Courtès @ 2021-10-29 12:09 UTC (permalink / raw)
  To: Greg Hogan; +Cc: guix-devel, Liliana Marie Prikler

Hi Greg,

Greg Hogan <code@greghogan.com> skribis:

> The cmake-build-system does defer to gnu-build-system, which calls `make
> test -jN`; however, CMake generated Makefile specifies 'test' as a single
> target (the Ninja generator suffers from the same issue) so `ctest` is run
> without parallelism.
>
> To run CMake tests with parallelism the cmake-build-system should directly
> call `ctest` with the configured parallelism (same '-jN' argument). The
> cmake-build-system's check method is essentially untouched from March, 2013
> (commit c6bded8a) so this issue has existed from the beginning.

OK, got it.

> I made an inelegant patch to my local guix repo, essentially replacing
> cmake-build-system:check with a copy of gnu-build-system:check and changing
> 'apply invoke "make" test-target' to 'apply invoke "ctest"'. This works,
> although the package I was working on requires parallel tests to be
> disabled. Once the expectation is set that code will be run serially it is
> difficult to change the default to parallel execution.
>
> It seems that we should at a minimum document the issue in
> cmake-build-system:check. We could patch cmake-build-system to enable test
> parallelism and explicitly disable that setting for packages which succeed
> before but fail after making the change. What do you think?

I think we could make the change you made—running ‘ctest’—in
‘core-updates’.  If it turns out to be safer, we can have
#:parallel-build? default to #f at the beginning in
‘cmake-build-system’.

How does that sound?

Thanks,
Ludo’.


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

* Re: Test parallelism with CMake
  2021-10-29 12:09   ` Ludovic Courtès
@ 2021-11-03 14:47     ` Greg Hogan
  2021-11-03 17:58       ` Liliana Marie Prikler
  0 siblings, 1 reply; 9+ messages in thread
From: Greg Hogan @ 2021-11-03 14:47 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: guix-devel, Liliana Marie Prikler

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

On Fri, Oct 29, 2021 at 8:09 AM Ludovic Courtès <ludo@gnu.org> wrote:

> Hi Greg,
>
> Greg Hogan <code@greghogan.com> skribis:
> > It seems that we should at a minimum document the issue in
> > cmake-build-system:check. We could patch cmake-build-system to enable
> test
> > parallelism and explicitly disable that setting for packages which
> succeed
> > before but fail after making the change. What do you think?
>
> I think we could make the change you made—running ‘ctest’—in
> ‘core-updates’.  If it turns out to be safer, we can have
> #:parallel-build? default to #f at the beginning in
> ‘cmake-build-system’.
>
> How does that sound?
>

Hi Liliana and Ludo’,

If the default is to disable parallel tests then it is unlikely that future
committers will be aware of, remember, and bother to enable the setting.
And once packages do override the default then there would be additional
future work to invert the default value.

I do not know how to obtain the list of packages using the
cmake-build-system.

The strategy looks to require multiple stages. First build and test the
packages and ignore current failures. As noted by Liliana, next build and
test the successful builds with ctest in serial. Hopefully no or few builds
are newly failing at this point. Lastly build and test with ctest in
parallel, and explicitly disable test parallelism for the failing builds.

Greg

[-- Attachment #2: Type: text/html, Size: 1933 bytes --]

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

* Re: Test parallelism with CMake
  2021-11-03 14:47     ` Greg Hogan
@ 2021-11-03 17:58       ` Liliana Marie Prikler
  0 siblings, 0 replies; 9+ messages in thread
From: Liliana Marie Prikler @ 2021-11-03 17:58 UTC (permalink / raw)
  To: Greg Hogan, Ludovic Courtès; +Cc: guix-devel

Hi Greg,

Am Mittwoch, den 03.11.2021, 10:47 -0400 schrieb Greg Hogan:
> If the default is to disable parallel tests then it is unlikely that
> future committers will be aware of, remember, and bother to enable
> the setting. And once packages do override the default then there
> would be additional future work to invert the default value.
I don't think it'd be that bad.  It'd just be a case of counting how
many packages do what, announcing the change and going through a c-u
cycle.

> I do not know how to obtain the list of packages using the cmake-
> build-system.
Grep is a good approximation, but you can iterate over all packages and
check.

> The strategy looks to require multiple stages. First build and test
> the packages and ignore current failures. As noted by Liliana, next
> build and test the successful builds with ctest in serial. Hopefully
> no or few builds are newly failing at this point. Lastly build and
> test with ctest in parallel, and explicitly disable test parallelism
> for the failing builds.
I think you can skip check two if you've independently verified for a
given package, that parallel ctest works.  What we would have to verify
now is whether indeed all (but perhaps a tiny minority of) cmake
packages test with ctest.

Cheers



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

* Re: Test parallelism with CMake
  2021-10-08 11:07 Greg Hogan
@ 2021-10-21 19:30 ` Ludovic Courtès
  0 siblings, 0 replies; 9+ messages in thread
From: Ludovic Courtès @ 2021-10-21 19:30 UTC (permalink / raw)
  To: Greg Hogan; +Cc: guix-devel

Hi Greg,

Greg Hogan <code@greghogan.com> skribis:

> As I read the source, cmake-build-system should by default both build and
> check with parallelism enabled. When I locally build a package only the
> build phase runs with parallelism and tests are being run in serial.
>
> When I run a manual build (stopping an in-process build run with '-K', then
> removing all files under the build directory, then copying and running the
> commands from the stopped build) I do not see a parallel build, nor do I
> see any parallelism passed by command or environment arguments (no '-j' or
> CMAKE_BUILD_PARALLEL_LEVEL).

Does CMake generate makefiles targets that would allow tests to run in
parallel?  How does that even work in CMake?

Thanks,
Ludo’.


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

* Test parallelism with CMake
@ 2021-10-08 11:07 Greg Hogan
  2021-10-21 19:30 ` Ludovic Courtès
  0 siblings, 1 reply; 9+ messages in thread
From: Greg Hogan @ 2021-10-08 11:07 UTC (permalink / raw)
  To: guix-devel

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

Guix,

As I read the source, cmake-build-system should by default both build and
check with parallelism enabled. When I locally build a package only the
build phase runs with parallelism and tests are being run in serial.

When I run a manual build (stopping an in-process build run with '-K', then
removing all files under the build directory, then copying and running the
commands from the stopped build) I do not see a parallel build, nor do I
see any parallelism passed by command or environment arguments (no '-j' or
CMAKE_BUILD_PARALLEL_LEVEL).

If this has been previously discussed please direct me to the earlier
discussion. Often my builds are fast (running in parallel) but tests are
slow (running in serial) and I am puzzled why any of this works the way it
does.

Greg

[-- Attachment #2: Type: text/html, Size: 928 bytes --]

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

end of thread, other threads:[~2021-11-03 17:59 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-09  7:56 Test parallelism with CMake Liliana Marie Prikler
2021-10-12 17:28 ` Greg Hogan
2021-10-22 13:10 ` Greg Hogan
2021-10-22 18:28   ` Liliana Marie Prikler
2021-10-29 12:09   ` Ludovic Courtès
2021-11-03 14:47     ` Greg Hogan
2021-11-03 17:58       ` Liliana Marie Prikler
  -- strict thread matches above, loose matches on Subject: below --
2021-10-08 11:07 Greg Hogan
2021-10-21 19:30 ` Ludovic Courtès

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