-
Notifications
You must be signed in to change notification settings - Fork 8
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
Enable multi-arch docker builds #2
Conversation
Thanks for the suggestion. I'd love to add ARM64 support. Unfortunately I've made extensive use of |
Hmm, so I'm not super familiar with the I will hopefully have time this weekend to dig in a smidge, see if I can test this properly :) |
The reference C implementation does indeed support ARM64 NEON. Unfortunately, the Go port does not: https://github.com/minio/simdjson-go
There’s an open issue regarding this: minio/simdjson-go#51
…On Fri, Jan 13 2023 at 8:47 PM, Daniel Carbone < ***@***.*** > wrote:
Hmm, so I'm not super familiar with the simdjson project, but according to
their homepage ( https://simdjson.org/about/ ) ARM NEON is supported.
I will hopefully have time this weekend to dig in a smidge, see if I can
test this properly :)
—
Reply to this email directly, view it on GitHub (
#2 (comment) ) ,
or unsubscribe (
https://github.com/notifications/unsubscribe-auth/AABAAZ53KUZ54JS6HQH2UCLWSIVXXANCNFSM6AAAAAAT25FI4E
).
You are receiving this because you commented. Message ID: <danielchalef/mrfparse/pull/2/c1382660758
@ github. com>
|
Ah, I see! They're using an assembly generator. Unfortunately looks like the generator itself has been archived :( Hmm...that's unfortunate. Would you be willing to accept a PR that allows for the use of the admittedly slower internal go json handling? I haven't had a look to check the feasibility of this, but it would be a stop-gap until a more efficient solution could be found. |
@dcarbone How about we implement this for development / test purposes? https://github.com/kiwicom/fakesimdjson Happy to accept a PR. |
Just as an update, i've not forgotten about this PR. I added Relevant PR here: kiwicom/fakesimdjson#2 |
Great to hear! |
closing in favor of #10 |
Hi there, me again :)
Another thing that would help adoption is to remove the hard-coded requirement of
amd64
(andlinux
), when building the binary either on-host or in-container.As a quick example, I removed the
GOOS
andGOARCH
vars from the dockerfile, then subsequently built and pushed this image to my public dockerhub here: https://hub.docker.com/r/dancarbone/danielchalef-mrfparse/tagsYou can see that there are two images present for this tag, one arm64, one amd64. This was done using the modified Dockerfile in this PR and the following command:
docker buildx build --platform linux/arm64,linux/amd64 -t dancarbone/danielchalef-mrfparse --push .
Proof of function:
Enabling multi-arch builds would let the ARM users on my team run this locally without the need to build it themselves, let alone have golang installed :)