-
Notifications
You must be signed in to change notification settings - Fork 544
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
Improve Tempo build options #4755
base: main
Are you sure you want to change the base?
Conversation
4bed0ca
to
e57eddf
Compare
GO_ENV+= GOAMD64=v2 | ||
endif | ||
ifeq ($(GOARCH),arm64) | ||
GO_ENV+= GOARM64=v8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
v8 is already the default, I'm still adding it for completeness
This enables the compiler to use instructions from more recent API versions
Reduces Tempo binary size by ~20%
GO111MODULE defaults to 'on' since Go 1.16
1c4a4da
to
5d222fc
Compare
|
||
.PHONY: tempo-query | ||
tempo-query: ## Build tempo-query | ||
GO111MODULE=on CGO_ENABLED=0 go build $(GO_OPT) -o ./bin/$(GOOS)/tempo-query-$(GOARCH) $(BUILD_INFO) ./cmd/tempo-query |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
afaik CGO_ENABLED is enabled by default
@@ -66,19 +79,20 @@ FILES_TO_JSONNETFMT=$(shell find ./operations/jsonnet ./operations/tempo-mixin - | |||
##@ Building | |||
.PHONY: tempo | |||
tempo: ## Build tempo | |||
GO111MODULE=on CGO_ENABLED=0 go build $(GO_OPT) -o ./bin/$(GOOS)/tempo-$(GOARCH) $(BUILD_INFO) ./cmd/tempo | |||
echo $(GOARCH) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
echo?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
echo
echo
echo
echo
echo
echo
in seriousness: lovely PR. easy, super safe improvement with some nice benches. i wonder when we'll be able to go to v3?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i wonder when we'll be able to go to v3?
I was talking about this with @stoewer and I think we can publish another build that's v3, called it something like optimised so folks who want can use v3.
another way we can do is, we can build v3 by default and publish another one called legacy
that's build with v2.
most of our users are using cloud providers, and most Nodes on public CSPs should support v3.
What this PR does:
This PR makes the following changes to Tempos build options:
-ldflags -w
. This reduces the tempo binary size by ~20%GO111MODULE=on
as this is the default since Go 1.16.The changes are inspired by the 2025 FOSDEM talk Build better Go release binaries
Benchmarks amd64 v1 vs v2 vs v3
Based on the benchmark results it would be interesting to find out if we can also use
GOAMD64=v3
. v3 corresponds to the AVX2 instruction set which was first introduced in 2013. There could still be some hardware around that does not support it.Checklist
CHANGELOG.md
updated - the order of entries should be[CHANGE]
,[FEATURE]
,[ENHANCEMENT]
,[BUGFIX]