checkPhase of buildGoModule is very slow

[Ma27]

Describe the bug

When running the default checkPhase of buildGoModule, it isn't parallelized at all: a for loop iterates through each directory where Go code was compiled and executes go test:

nixpkgs/pkgs/build-support/go/module.nix

Lines 276 to 278 in b8b232a

	for pkg in $(getGoDirs test); do
	buildGoDir test "$pkg"
	done

This is incredibly slow, for instance the last build of Grafana on master spent 20min on the test suite: https://cache.nixos.org/log/ninyms7zfs2naq6qph1ysb6zgib1xxp3-grafana-10.3.1.drv

The actual runtime of each case doesn't seem that high, so the biggest bottleneck is probably the compilation of test code. This should be parallelized.

Steps To Reproduce

Steps to reproduce the behavior:

1. nix-build -A grafana.

Expected behavior

Being able to run tests truly in parallel.

Screenshots

n/a

Additional context

n/a

Notify maintainers

cc @kalbasit @Mic92 @zowoq

Metadata

Please run nix-shell -p nix-info --run "nix-info -m" and paste the result.
n/a

---

Add a 👍 reaction to issues you find important.

[Mic92]

Maybe something like xargs -P $NIX_JOBS ... would help to parallelize.

[kalbasit]

go build and go test do accept multiple dirs so we could technically just do buildGoDir test $(getGoDirs test) assuming we've modified buildGoDir to accept multiple directories and pass them as arguments to go $cmd

[qbit]

#284568 will help address this.

[Mic92]

go build and go test do accept multiple dirs so we could technically just do buildGoDir test $(getGoDirs test) assuming we've modified buildGoDir to accept multiple directories and pass them as arguments to go $cmd

Could we not make the argument list too big for large projects this way?

[kalbasit]

go build and go test do accept multiple dirs so we could technically just do buildGoDir test $(getGoDirs test) assuming we've modified buildGoDir to accept multiple directories and pass them as arguments to go $cmd

Could we not make the argument list too big for large projects this way?

That's possible but I wonder how probable that is. @katexochen if you get a chance, can you try your PR to build Kubernetes or some other large project?

[katexochen]

Could we not make the argument list too big for large projects this way?

I can't think of a reason this shouldn't work with big argument lists.

katexochen if you get a chance, can you try your PR to build Kubernetes or some other large project?

Kubernetes package isn't affected by this, as its derivation overwrites the buildPhase and also don't execute tests. Any other idea for larger projects to test?

[katexochen]

Performance of grafana build master vs. #284568:

master:

nix log /nix/store/h7aac712pcpx4481k0xfxklj6yd9k2kq-grafana-10.3.3.drv | grep completed
configurePhase completed in 1 minutes 27 seconds
buildPhase completed in 5 minutes 54 seconds
checkPhase completed in 18 minutes 25 seconds

on #284568:

nix log /nix/store/avnyk3vs3ig5mxc3hkj12r3xqfbcq37x-grafana-10.3.1 | grep completed
configurePhase completed in 1 minutes 26 seconds
buildPhase completed in 3 minutes 2 seconds
checkPhase completed in 11 minutes 57 seconds

But the package could also use some refactoring and cleanup. Especially, I would recommended to set subPackages to only build what is necessary (I'll add that to the docs as soon as I find the time).

[Ma27]

I can't think of a reason this shouldn't work with big argument lists.

Your patch basically does go build $(go list ...), correct? If the return value of go list is too long, this will break.

Kubernetes package isn't affected by this, as its derivation overwrites the buildPhase and also don't execute tests. Any other idea for larger projects to test?

I'd probably use kubernetes and comment out the build phase (and probably add a set -x in preBuild). If the argument list is too long, you'll notice.
If kubernetes isn't affected by this, I think it's OK to leave it in our patch as-is. Alternatively, perhaps something like xargs -P$JOBS?

[Ma27]

But the package could also use some refactoring and cleanup. Especially, I would recommended to set subPackages to only build what is necessary (I'll add that to the docs as soon as I find the time).

So I reduced it down to pkg//cmd/grafana{,-{server,cli}} and the tests are still passing which is an indicator to me that this is sufficient.
OTOH this also means that no tests at all are executed. Any suggestions on how to still 'run the test-suite?

[katexochen]

But the package could also use some refactoring and cleanup. Especially, I would recommended to set subPackages to only build what is necessary (I'll add that to the docs as soon as I find the time).

So I reduced it down to pkg//cmd/grafana{,-{server,cli}} and the tests are still passing which is an indicator to me that this is sufficient. OTOH this also means that no tests at all are executed. Any suggestions on how to still 'run the test-suite?

That's what #284568 does.