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

Avoid eagerly doing unnecessary string formatting #73

Merged
merged 2 commits into from
Nov 21, 2024

Conversation

ludfjig
Copy link
Contributor

@ludfjig ludfjig commented Nov 20, 2024

Switch from .ok_or to .ok_or_else do only perform the string formatting in the failing case.
Benchmarks show that time to create an uninitialized sandbox goes from 400µs to about 330µs (-17%) on my local machine

Also adds an amd tag that was forgotten in PR #74

@ludfjig ludfjig changed the title Avoid eager unnecessary string formatting Avoid eagerly evaluating string formatting Nov 20, 2024
@ludfjig ludfjig changed the title Avoid eagerly evaluating string formatting Avoid eagerly doing unnecessary string formatting Nov 20, 2024
@ludfjig ludfjig force-pushed the lazy_eval_error branch 2 times, most recently from 41a3fb3 to 81288e9 Compare November 20, 2024 06:49
dblnz
dblnz previously approved these changes Nov 20, 2024
Copy link
Contributor

@dblnz dblnz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch

simongdavies
simongdavies previously approved these changes Nov 20, 2024
Signed-off-by: Ludvig Liljenberg <lliljenberg@microsoft.com>
@ludfjig ludfjig dismissed stale reviews from simongdavies and dblnz via 7d5ba05 November 20, 2024 22:47
Signed-off-by: Ludvig Liljenberg <lliljenberg@microsoft.com>
@ludfjig ludfjig enabled auto-merge (squash) November 21, 2024 00:55
@ludfjig ludfjig mentioned this pull request Nov 21, 2024
@ludfjig ludfjig merged commit a165293 into hyperlight-dev:main Nov 21, 2024
23 checks passed
@ludfjig ludfjig deleted the lazy_eval_error branch November 21, 2024 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants