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.

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

[CI] musl build updates #256

Merged
merged 22 commits into from
Nov 6, 2024
Merged

[CI] musl build updates #256

merged 22 commits into from
Nov 6, 2024

Conversation

iamazeem
Copy link
Collaborator

@iamazeem iamazeem commented Nov 1, 2024

  • Separate job for musl with alpine:latest container
  • Cleanup of Makefiles and shell scripts

Signed-off-by: Azeem Sajid [email protected]

@iamazeem iamazeem added the enhancement New feature or request label Nov 1, 2024
@iamazeem iamazeem self-assigned this Nov 1, 2024
@iamazeem iamazeem marked this pull request as draft November 1, 2024 17:45
@liquidaty
Copy link
Owner

@iamazeem could you also double-check if these contemplated changes have any measurable impact on the performance benchmarks?

@iamazeem
Copy link
Collaborator Author

iamazeem commented Nov 2, 2024

@liquidaty: Here are the results:


Benchmarks (WITHOUT_SIMD=true)

  • Timestamp UTC: 2024-11-02T07:23:57Z

Releases Used

Results

count

1 | zsv : real 0.17 user 0.16 sys 0.00
2 | zsv : real 0.17 user 0.17 sys 0.00
3 | zsv : real 0.17 user 0.17 sys 0.00
4 | zsv : real 0.17 user 0.16 sys 0.01
5 | zsv : real 0.18 user 0.17 sys 0.00
6 | zsv : real 0.17 user 0.17 sys 0.00
1 | xsv : real 0.08 user 0.07 sys 0.01
2 | xsv : real 0.08 user 0.08 sys 0.00
3 | xsv : real 0.08 user 0.07 sys 0.01
4 | xsv : real 0.08 user 0.08 sys 0.00
5 | xsv : real 0.08 user 0.08 sys 0.00
6 | xsv : real 0.08 user 0.07 sys 0.01
1 | tsv : real 0.08 user 0.08 sys 0.00
2 | tsv : real 0.08 user 0.09 sys 0.00
3 | tsv : real 0.09 user 0.06 sys 0.02
4 | tsv : real 0.08 user 0.08 sys 0.00
5 | tsv : real 0.08 user 0.07 sys 0.01
6 | tsv : real 0.09 user 0.08 sys 0.00

select

1 | zsv : real 0.39 user 0.36 sys 0.02
2 | zsv : real 0.42 user 0.41 sys 0.00
3 | zsv : real 0.39 user 0.39 sys 0.00
4 | zsv : real 0.39 user 0.37 sys 0.01
5 | zsv : real 0.39 user 0.39 sys 0.00
6 | zsv : real 0.38 user 0.38 sys 0.00
1 | xsv : real 0.36 user 0.36 sys 0.00
2 | xsv : real 0.36 user 0.35 sys 0.01
3 | xsv : real 0.36 user 0.35 sys 0.00
4 | xsv : real 0.36 user 0.35 sys 0.00
5 | xsv : real 0.36 user 0.36 sys 0.00
6 | xsv : real 0.36 user 0.34 sys 0.01
1 | tsv : real 0.11 user 0.10 sys 0.00
2 | tsv : real 0.11 user 0.10 sys 0.00
3 | tsv : real 0.11 user 0.10 sys 0.00
4 | tsv : real 0.11 user 0.10 sys 0.00
5 | tsv : real 0.11 user 0.10 sys 0.00
6 | tsv : real 0.11 user 0.08 sys 0.02

Benchmarks (WITHOUT_SIMD=false)

  • Timestamp UTC: 2024-11-02T07:52:20Z

Releases Used

Results

count

1 | zsv : real 0.03 user 0.03 sys 0.00
2 | zsv : real 0.03 user 0.03 sys 0.00
3 | zsv : real 0.03 user 0.03 sys 0.00
4 | zsv : real 0.03 user 0.03 sys 0.00
5 | zsv : real 0.03 user 0.03 sys 0.00
6 | zsv : real 0.03 user 0.03 sys 0.00
1 | xsv : real 0.08 user 0.07 sys 0.00
2 | xsv : real 0.08 user 0.07 sys 0.01
3 | xsv : real 0.08 user 0.07 sys 0.00
4 | xsv : real 0.08 user 0.07 sys 0.00
5 | xsv : real 0.08 user 0.07 sys 0.01
6 | xsv : real 0.08 user 0.07 sys 0.00
1 | tsv : real 0.08 user 0.07 sys 0.00
2 | tsv : real 0.08 user 0.08 sys 0.00
3 | tsv : real 0.08 user 0.08 sys 0.00
4 | tsv : real 0.08 user 0.08 sys 0.00
5 | tsv : real 0.08 user 0.08 sys 0.00
6 | tsv : real 0.08 user 0.08 sys 0.00

select

1 | zsv : real 0.25 user 0.23 sys 0.01
2 | zsv : real 0.25 user 0.25 sys 0.00
3 | zsv : real 0.25 user 0.24 sys 0.01
4 | zsv : real 0.25 user 0.25 sys 0.00
5 | zsv : real 0.25 user 0.25 sys 0.00
6 | zsv : real 0.25 user 0.24 sys 0.00
1 | xsv : real 0.36 user 0.35 sys 0.00
2 | xsv : real 0.37 user 0.35 sys 0.01
3 | xsv : real 0.37 user 0.36 sys 0.00
4 | xsv : real 0.36 user 0.34 sys 0.02
5 | xsv : real 0.36 user 0.34 sys 0.01
6 | xsv : real 0.37 user 0.36 sys 0.00
1 | tsv : real 0.11 user 0.10 sys 0.00
2 | tsv : real 0.11 user 0.10 sys 0.00
3 | tsv : real 0.11 user 0.10 sys 0.00
4 | tsv : real 0.11 user 0.11 sys 0.00
5 | tsv : real 0.11 user 0.10 sys 0.00
6 | tsv : real 0.11 user 0.10 sys 0.00

Notes

@iamazeem
Copy link
Collaborator Author

iamazeem commented Nov 2, 2024

A binary built with SIMD instructions fails with "illegal instruction" error on platforms where those instructions aren't available.
We don't necessarily have to publish the non-SIMD release.
The WITHOUT_SIMD flag is just there to handle such scenarios whenever needed.

@liquidaty
Copy link
Owner

@iamazeem I'm not aware of any chips from 2018+ that do not have some sort of SIMD capabilities, so is it possible that WITHOUT_SIMD is using unnecessarily conservative assumptions, and the better solution would be to better tailor the build to properly discover and use the available SIMD instructions? Alternatively is there a limitation to the musl compiler's ability to generate correct SIMD instructions for varied platforms?

@liquidaty
Copy link
Owner

A binary built with SIMD instructions fails with "illegal instruction" error on platforms where those instructions aren't available. We don't necessarily have to publish the non-SIMD release. The WITHOUT_SIMD flag is just there to handle such scenarios whenever needed.

Disregard my prior comment, as I did not closely read the above-- thank you, makes sense. Let's include the WITHOUT_SIMD for CI, but not actually publish it (at least for now) upon release. Thank you!

@iamazeem iamazeem marked this pull request as ready for review November 3, 2024 09:44
@iamazeem iamazeem requested a review from liquidaty November 3, 2024 09:45
@iamazeem
Copy link
Collaborator Author

iamazeem commented Nov 4, 2024

@iamazeem iamazeem merged commit 522e5d4 into main Nov 6, 2024
11 checks passed
@iamazeem iamazeem deleted the ci-musl-build-updates branch November 6, 2024 05:08
@iamazeem
Copy link
Collaborator Author

iamazeem commented Nov 6, 2024

Benchmarks from the latest run on main after merge:
https://github.com/liquidaty/zsv/actions/runs/11697708382

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants