-
Notifications
You must be signed in to change notification settings - Fork 263
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
Statically linked spin #2101
Statically linked spin #2101
Conversation
Signed-off-by: Sven Pfennig <[email protected]>
Signed-off-by: Sven Pfennig <[email protected]>
Signed-off-by: Sven Pfennig <[email protected]>
Signed-off-by: Rajat Jindal <[email protected]>
7bf1319
to
76ff994
Compare
Signed-off-by: Rajat Jindal <[email protected]>
Signed-off-by: Rajat Jindal <[email protected]>
da8a5bf
to
18d1789
Compare
While I wouldn't necessarily expect there to be problems, we should verify that these builds don't meaningfully regress performance. Musl isn't the fastest libc out there, and in particular its malloc implementation isn't fast, so that might result in unexpected regressions if we have hot code somewhere in Spin where that matters. (And if all of this isn't mitigated by Rust's Musl target anyway.) |
Hi @tschneidereit, thank you for your feedback. Do you happen to have any suggestions about what could be the alternative here? I am happy to explore other options here. |
also, just to ensure we are on same page, this PR adds additional release assets for statically linked Spin binaries (and does not replace existing releases binaries). |
Oh sorry, I should've been clearer: I agree that having these builds makes sense! I also don't really have an alternative as such. That doesn't mean that we shouldn't test performance of this configuration vs others though. If we were to discover meaningful regressions, there might be things we can do to address those without moving away from this target. For example, we could look into using a different allocator, or we could potentially change things in Spin to not be as affected by musl's lower performance. |
For openshift integration we need statically linked binary of spin. This PR adds the required changes for that.
I think we also need to attach these artifacts to releases, I will look at it next (will add commits to this PR)
cc @0xE282B0