-
Notifications
You must be signed in to change notification settings - Fork 654
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
building with race detection (@io_bazel_rules_go//go/config:race
) is very slow especially if using Go binaries to generate code
#3218
Comments
Just for clarity, the linked issue is cockroachdb/cockroach#81314 as the link in the OP text is broken. |
@rickystewart Given that you are experiencing the pain of the issue, would you be interested in submitting a PR to fix the |
We encountered this issue in the Figma monorepo.
For us, this is the primary issue, as building the stdlib causes Bazel to get OOM-killed on our CI executor (4GB RAM), even with
|
Bazel now has an API for adding resource requirements to actions. It is still experimental, but we could optionally use it for stdlib actions. I am working on fixing the issue that the stdlib is rebuilt more often than necessary, but that may also require flags. |
We ran into this as well recently. The speed is not an issue thanks to RBE. However, we got this error when race is enabled via a flag. ERROR: /private/var/tmp/_bazel_sluongng/06e573a93bc2d6a9cad4ad41f00b4310/external/io_bazel_rules_go/BUILD.bazel:42:7: in stdlib rule @@io_bazel_rules_go//:stdlib:
Traceback (most recent call last):
File "/private/var/tmp/_bazel_sluongng/06e573a93bc2d6a9cad4ad41f00b4310/external/io_bazel_rules_go/go/private/rules/stdlib.bzl", line 33, column 20, in _stdlib_impl
go = go_context(ctx)
File "/private/var/tmp/_bazel_sluongng/06e573a93bc2d6a9cad4ad41f00b4310/external/io_bazel_rules_go/go/private/context.bzl", line 458, column 20, in go_context
mode = get_mode(ctx, toolchain, cgo_context_info, go_config_info)
File "/private/var/tmp/_bazel_sluongng/06e573a93bc2d6a9cad4ad41f00b4310/external/io_bazel_rules_go/go/private/mode.bzl", line 107, column 13, in get_mode
fail("race instrumentation can't be enabled when cgo is disabled. Check that pure is not set to \"off\" and a C/C++ toolchain is configured.")
Error in fail: race instrumentation can't be enabled when cgo is disabled. Check that pure is not set to "off" and a C/C++ toolchain is configured.
ERROR: /private/var/tmp/_bazel_sluongng/06e573a93bc2d6a9cad4ad41f00b4310/external/io_bazel_rules_go/BUILD.bazel:42:7: Analysis of target '@@io_bazel_rules_go//:stdlib' failed The issue goes away if I were to remove the go_binary included in Setting I think a surefire way to fix this is to provide a mechanism to enforce I was looking into |
I created a quick POC of the transition mentioned above ☝️ here buildbuddy-io/buildbuddy#5748 if anyone wants a quick solution. |
@sluongng I would support a PR that simply ignores the value of |
Regarding the original issue (and @rickystewart): I now think that we should probably just disable More generally, we would want to revisit and specify how our build settings should change under the exec transition. For example, I see two ways to achieve this today:
Contributions would be very welcome. |
Would ignoring Looking at this rules_go/go/private/actions/stdlib.bzl Lines 127 to 131 in 30099a6
It seems like the diff would be whether we gonna > GOCACHE=/tmp/ go install -x -race runtime/cgo` and it seems to just work? 🤔 |
oh nvm, I misread it, the diff is when we have both So the question would be, are there any use cases for such a combination: i.e. setting up a race-detection test with My guess is it's fine to "ignore the value of So I think the additional transition would still be a surefire approach to get the "expected" result in this case. |
Over at Cockroach we are having this issue where building a
go_test
with--@io_bazel_rules_go//go/config:race
takes a very long time. The fact that the stdlib needs to be recompiled with race detection is a problem, but more relevant seems to be that building a target with race detection invalidates the cache for allgo_binary
s used to produce that target, includinggo_binary
s that are built asexec_tools
for agenrule
, and causes all of them to be re-built with race detection. If you use Go code to generate other Go code (as we do), the problem is now twofold:go_binary
for code generation cannot be reused, so the binary (and presumably all or most of its dependencies) needs to be rebuilt with race detection enabled (a slower build process in and of itself).Since our code generators produce identical output regardless of whether race detection is enabled, it's very wasteful to re-build the code generators and then run a much slower version of the binary as part of the race build.
Presumably this behavior is OK by default, but there should be a workaround. I have tried updating the
go_binary
declaration for these code generating binaries to haverace = "off"
but it appears to have no effect -- setting the value on the command line overrides everything inBUILD
files. It would be sufficient if I could have a way to specify that certain targets really do not need to be built with race detection despite the command-line flags.What version of rules_go are you using?
0.32
What version of gazelle are you using?
0.25
What version of Bazel are you using?
5.1.0
Does this issue reproduce with the latest releases of all the above?
Yes
What operating system and processor architecture are you using?
Darwin ARM64
Any other potentially useful information about your toolchain?
(None)
What did you do?
Tried setting
race = "off"
for code-generatinggo_binary
target when building a target that depends on that target as anexec_tool
.What did you expect to see?
Ideally the code-generating binary would not be built with race-detection especially if I set
race = "off"
manually.What did you see instead?
Code-generating binary is built with race detection, invalidating the cache and slowing the entire build down.
The text was updated successfully, but these errors were encountered: