-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Upgrade to Go 1.17 #8640
Upgrade to Go 1.17 #8640
Conversation
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
@@ -126,3 +126,69 @@ require ( | |||
k8s.io/code-generator v0.17.3 | |||
sigs.k8s.io/yaml v1.2.0 | |||
) | |||
|
|||
require ( |
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.
It's weird to have two require
blocks. Will a go mod tidy
clean this up?
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.
No, this is the "New Way" ™️ in 1.17, specifically:
If a module specifies go 1.17 or higher in its go.mod file, its go.mod file now contains an explicit require directive for every module that provides a transitively-imported package. (In previous versions, the go.mod file typically only included explicit requirements for directly-imported packages.)
Because the number of explicit requirements may be substantially larger in an expanded Go 1.17 go.mod file, the newly-added requirements on indirect dependencies in a go 1.17 module are maintained in a separate require block from the block containing direct dependencies.
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.
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
Tests are all green after building the new docker bootstrap image. Waiting for tomorrow morning to see the benchmarking results. |
The upgrade presents no significant performance improvement or regression: https://benchmark.vitess.io/compare?r=d10312297ffd33b144d742aba62fe5365c6b5019&c=645f06caf8336f2ab25f8fab101b11776914ed3b |
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'm a bit disappointed that the new register allocation engine in the compiler doesn't appear to have a measurable effect in the performance of the whole system. I managed to see some improvements on micro-benchmarks, so it's a bit of a bummer that this is not being propagated to macros.
Paraphrasing offline conversation with @frouioui: |
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.
Nice work @frouioui! Thank you for taking this on.
Description
Bumping the Go version of the project to Go 1.17.
Checklist