Skip to content

Conversation

@alecloudenback
Copy link
Collaborator

No description provided.

@MatthewCaseres
Copy link
Contributor

Reasons I hesitate to merge

CI Time

CI time is a bit painful at the moment. This PR takes CI time from 5 minutes to 10 minutes.

Uncertainty of benefits

I don't know the exact mechanics behind how parallel runs work. Do they happen on the same machine, on multiple machines with the same specs, are other people on the same running their cloud actions regardless of if I am running mine in parallel?

I have observed the relative orders to be generally stable in the current approach. We can do some historical analysis and verify this if it is a concern.

Desire to eventually run on dedicated hardware

Ideally all benchmarks are run on dedicated hardware. And we would test across a variety of platforms. This is not cost effective or practical to run these variety of dedicated hardware in GitHub actions.

I imagine an annual performance testing event. In that case, the README will start with the results of the annual test and then below will be the preliminary tests for the upcoming year that ran in the CI. I feel the CI is just a nice way to say "the code runs".

@alecloudenback
Copy link
Collaborator Author

alecloudenback commented Mar 2, 2024

  • I think that a single action is generally run on one virtual machine so benchmarks, especially those that will saturate available threads, will inherently compete against each other.
  • the timing on the new Julia benchmark was ~30 microseconds on the PR CI, but master branch now shows 49 microseconds which makes me thinks there is enough noise or inconsistency with runs that we should make attempts to minimize any inconsistency
  • The runtime on this PR was still only ~10 minutes, compared to ~6 minutes on master. IMO four minutes is worth it if the benchmarks are to be more trustworthy

@MatthewCaseres
Copy link
Contributor

I'm convinced

@MatthewCaseres MatthewCaseres merged commit b90ff0a into actuarialopensource:main Mar 2, 2024
@alecloudenback alecloudenback deleted the ci2 branch March 2, 2024 03:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants