Use Zig to build aarch64 binaries #1249
Conversation
|
|
|
BTW, it also helps reduce the build time. The current workflow costs about 25 minutes for 3 platforms totally:
Using Zig only cost 12 minutes for 3 platforms totally:
|
| var version = if (std.os.getenv("VERSION")) |v| v else "unknown"; | ||
|
|
||
| var targets = [_]BinaryTarget{ | ||
| // for now, let's only build aarch64 binaries |
There was a problem hiding this comment.
thanks.. this indeed sound like a good potential simplification for the future, but base upon the current level of popularity of the zig compiler, i'd appreciate to keep them separate for now. :)
Although, curious if you know whether the x86 binaries built this way are in what ways different/similar to the ones we build today? e.g. in terms of binary size, optimization, etc?
There was a problem hiding this comment.
The most significant difference is that in the existing workflow we build x86_64 using GCC, and Zig builds binaries using Clang internally (you can imagine Zig as a Clang wrapper). I think both of them are well optimized since Clang is also a very popular compiler. There are some binary size differences, but personally I'm OK with them.
Binary sizes with Zig:
protoc-gen-grpc-web-unknown-darwin-x86_64 6.7M
protoc-gen-grpc-web-unknown-linux-x86_64 1.8M
protoc-gen-grpc-web-unknown-windows-x86_64.exe 4.3M
Binary sizes with current workflow:
There was a problem hiding this comment.
ohh that's very cool to know that Zig is Clang based! Thanks a lot for the explanation! The binary sizes do seem to be around the same ballpark too so that's great to know too! :)
Maybe we'll consider moving all to Zig later if Zig turns out be be working well and stable :)
| exe.defineCMacro("HAVE_PTHREAD", "1"); | ||
| exe.addCSourceFiles(&[_][]const u8{ | ||
| // libprotobuf_lite | ||
| "../../../../../third_party/protobuf/src/google/protobuf/any_lite.cc", |
There was a problem hiding this comment.
Could you provide some context (and inline comments) on how to generate/maintain this list of files?
I'm a bit concerned about the maintenance overhead of this list. Any potential ways of improving it somehow?
There was a problem hiding this comment.
I copied the list from third_party/protobuf/cmake/libprotobuf-lite.cmake, third_party/protobuf/cmake/libprotobuf.cmake and third_party/protobuf/cmake/libprotoc.cmake, without any modifications.
I agree it is a pain that we need to keep the list up-to-date, but it is necessary when doing cross-compiling(we need to compile dependencies from source code as well).
There was a problem hiding this comment.
Thanks for adding the comments!
By any chance this can be parsed dynamically from the cmake files? If it's possible but not to be done for now, could you add a comment on the code on how this can be potentially improved? Thanks!
There was a problem hiding this comment.
It is possible, but personally, I think the complexity is too high for a build script (Do we need to write tests for it?), so I would prefer to copy the list manually.
There was a problem hiding this comment.
Yeah, i think it's a tradeoff between complexity v.s. maintenance overhead.
Test is probably not required because regardless of whether the manual or scripted way, compilation should fail if headers are not properly provided..
This is ok for now.. just my thoughts :)
|
Thanks so much for the contrib again! :) |
|
Hmm i just tried to test run the workflow but it failed.. 😅 https://github.com/grpc/grpc-web/actions/runs/2449921379 Any ideas why? :) |
|
Sorry I changed the Zig setup step to manually downloading, and I only tested it in a local Docker container, which is running with the root permission, however, we don't have the root permission in the GitHub Actions. A quick fix would be to add Sorry for that again. |
Aha no worries at all! Thanks for fixing it! 😃 |
|
I have tested building the ARM binaries again and it worked! Thanks again for the contrib, @hronro! |
Fix #1159
I've tested macOS/Linux binaries by using them with a helloworld.proto file, and there is no issue with them. I even tested x86_64 binaries produced by Zig even if they are not included in the PR. Windows binaries are not tested yet since I don't have a Windows machine.
Feel free to check the CI running result in my forked repo: https://github.com/hronro/grpc-web/actions/runs/2413657979