search

bazelbuild / rules_go

Go rules for Bazel
Apache License 2.0
1.35k stars 635 forks source link

Allow pure without purego #3959

Open zecke opened 2 weeks ago

zecke commented 2 weeks ago

What version of rules_go are you using?

0.48.0

What version of gazelle are you using?

0.36.0

What version of Bazel are you using?

bazel 6.4.x

Does this issue reproduce with the latest releases of all the above?

yes

What operating system and processor architecture are you using?

Linux

Any other potentially useful information about your toolchain?

The application is built with --@io_bazel_rules_go//go/config:pure to have no dependencies on GLIBC (or any other c library) as this allows building minimal container images.

What did you do?

Upgraded an application that uses google.golang.org/protobuf to the latest release of rules_go.

What did you expect to see?

No changes in performance.

What did you see instead?

The performance of the application has greatly reduced. Enabling "pure" now implies not using pointer arithmetic (https://github.com/search?q=repo%3Aprotocolbuffers%2Fprotobuf-go%20purego&type=code).

While it seems possible to add gotags in a go_binary it doesn't seem possible to take them away. Using cgo = False will still produce a dynamically linked executable (and require glibc at runtime).

fmeum commented 2 weeks ago

@mattyclarkson I think we (at least I) missed the interaction with unsafe. Rereading the Go thread on this, purego disables:

We might have to roll back as not having a C++ toolchain doesn't mean you don't want (or can't get) unsafe.

What's the situation for circl? Could we add noasm in addition to purego and only enable the former with pure?

mattyclarkson commented 2 weeks ago

What's the situation for circl? Could we add noasm in addition to purego and only enable the former with pure?

The upstream change I did for circl was that purego meant exactly that: nothing but pure go.

I didn't even know that there was a Go flavour of asssembly 😊 If that is what is in curve_amd64.s, then the purego tag at the top of that file is incorrect. I can put up a correction to include Go assembly in the build.

We might have to roll back

I'm OK to rollback to fix the performance regression. I feel the fix is upstream though.

not having a C++ toolchain doesn't mean you don't want (or can't get) unsafe.

As far as I understand it, unless a project respects the purego tag and removes files that use unsafe based on that, setting pure = "yes" should not negate using unsafe.

Could we add noasm in addition to purego and only enable the former with pure?

In rules_go we have pure = "{auto,on,off}" and cgo = {True,False}.

This is the current setup for the build flags based on what is enabled in the rules_go target:

rules_go cgo = True cgo = False
pure = "on" purego/cgo purego
pure = "off" |cgo`
pure = "auto" |cgo`

Note: cgo build flag is implicitly toggled if CGO_ENABLED={0,1}.

Projects could enable C toolchain ASM when both purego / cgo are enabled? Go lang assembly can be included at all times. unsafe can be used at all times.

I'm not a GoLang expert so unsure how these build variant parts interact.

zecke commented 2 weeks ago

@mattyclarkson I think we (at least I) missed the interaction with unsafe. Rereading the Go thread on this, purego disables:

  • ASM, which may require a C toolchain (not entirely sure about this, does this depend on whether the Go flavor of ASM is used?)

I don't think pure ASM will generally require a C toolchain. Let's say I use Go's assembly flavor to implement some code that needs to be "constant time". We will use some of the CPU architecture underlying instructions without having a dependency on external libraries. When it comes to linking Go's internal linker might try to load libraries from the toolchain (e.g. libgcc).

  • unsafe, which doesn't need a C toolchain

Indeed. This is where the performance degradation stems from.

  • CGo

Yes. That directly invokes the compiler and has external dependencies.

We might have to roll back as not having a C++ toolchain doesn't mean you don't want (or can't get) unsafe.

What's the situation for circl? Could we add noasm in addition to purego and only enable the former with pure?

zecke commented 2 weeks ago

he current setup for the build flags based on what is enabled in the rules_go target:

rules_go cgo = True cgo = False pure = "on" purego/cgo purego pure = "off"cgo pure = "auto" cgo
Note: cgo build flag is implicitly toggled if CGO_ENABLED={0,1}.

Projects could enable C toolchain ASM when both purego / cgo are enabled? Go lang assembly can be included at all times. unsafe can be used at all times.

When cgo is enabled the resulting binary will have a dependency on the C library. I think adding a row to the table on whether on Linux the resulting go binary has external dependencies would be helpful.