Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feature(releaser): better goreleaser with arm support and optimized AMD64 #428

Merged
merged 1 commit into from
Oct 1, 2024
Merged

feature(releaser): better goreleaser with arm support and optimized AMD64 #428

merged 1 commit into from
Oct 1, 2024

Conversation

CodeLieutenant
Copy link
Contributor

@CodeLieutenant CodeLieutenant commented Sep 30, 2024 •
edited
Loading

  • ARM64 Builds
  • Optimized x86_64 Build (Go compiler can use SSE3, SSE4.2 and AVX2 instructions)
  • Goreleaser now builds ARM and x86_64 binaries and docker images
  • Using docker manifest to generate multi-platform docker images (combine arm and x86_64 images)

@CodeLieutenant CodeLieutenant added the enhancement New feature or request label Sep 30, 2024
@CodeLieutenant CodeLieutenant self-assigned this Sep 30, 2024
dkropachev
dkropachev requested changes Sep 30, 2024
View reviewed changes
Copy link
Collaborator

@dkropachev dkropachev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add brief list of what has been changed and why, to the PR description.

.goreleaser.yml Outdated
@@ -2,34 +2,84 @@ version: 2

env:
- GO111MODULE=on
- GOARM64=v8.0,lse,crypto
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add a comment why exactly lse, crypto

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LSE is not needed, but crypto is, as gemini is using crypto random generator, and if we have that ext on arm, we can use hardware acceleration

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, please add a comment

.github/workflows/release.yml
@@ -25,6 +25,9 @@ jobs:
with:
go-version: '1.23'

- name: Set up QEMU
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need qemu?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are building cross platform for docker, we need QEMU

Copy link
Collaborator

@dkropachev dkropachev Sep 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add a comment

fruch
fruch approved these changes Sep 30, 2024
View reviewed changes
Copy link
Collaborator

@fruch fruch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

But it's not really that urgent for us to have arm images.

No one is gonna use them anytime soon.

@CodeLieutenant
Copy link
Contributor Author

But it's not really that urgent for us to have arm images.

Arm images are mostly side-effects in this pull request, there are some issues in goreleaser (mainly deprications) that are fixed, and optimized builds for AMD64 (since gemini is running on AWS, we can allow go to use more specialized instructions (SIMD), as AWS instances have modern CPUs

@CodeLieutenant CodeLieutenant marked this pull request as ready for review October 1, 2024 08:12
@fruch
Copy link
Collaborator

fruch commented Oct 1, 2024

But it's not really that urgent for us to have arm images.

Arm images are mostly side-effects in this pull request, there are some issues in goreleaser (mainly deprications) that are fixed, and optimized builds for AMD64 (since gemini is running on AWS, we can allow go to use more specialized instructions (SIMD), as AWS instances have modern CPUs

again, optimization which is nice to have

but we our focus with gemini for now, should be on it's functional behavior.
only after that we'll be able to focus on its performance

@dkropachev dkropachev merged commit 4d348b9 into scylladb:master Oct 1, 2024
4 of 5 checks passed
@CodeLieutenant CodeLieutenant deleted the feat/arm-builds branch October 1, 2024 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fruch fruch fruch approved these changes

@dkropachev dkropachev dkropachev approved these changes

Assignees

@CodeLieutenant CodeLieutenant

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@CodeLieutenant @fruch @dkropachev