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

Handle seconds=Inf and very high seconds args gracefully (unbounded runtime if samples and evals are specified, otherwise throw) #105

Merged
merged 5 commits into from
Sep 13, 2024

Conversation

LilithHafner
Copy link
Owner

@LilithHafner LilithHafner commented Jun 5, 2024

Fixes #104

  • handle seconds=Inf gracefully
  • handle very high seconds gracefully
  • handle time_ns() being close to the top of its range and overflowing mid-benchmark gracefully (with no errors)
  • add tests for all of the above

@LilithHafner
Copy link
Owner Author

RegressionTests

  • Increased runtime with very low runtime budget
  • Decreased correlation between true runtime and reported runtime

Normal tests:

runtime = @elapsed res = @be sleep(.1) seconds=.05
@test runtime < .2 # hopefully this is not going to get too many false positives

fails on 1.6

@LilithHafner
Copy link
Owner Author

Lots of little performance regressions and perf test failures, but all are tiny in magnitude so it's worth merging this bugfix. The regression test failures will be marked as accepted once this merges and the non-regression test failures were already flakey.

@LilithHafner LilithHafner merged commit c262e73 into main Sep 13, 2024
7 of 13 checks passed
@LilithHafner LilithHafner deleted the lh/long-runtime branch September 13, 2024 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

seconds=Inf does not work
1 participant