-
Notifications
You must be signed in to change notification settings - Fork 648
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
Check performance of gas with i64 #1884 #2062
Check performance of gas with i64 #1884 #2062
Conversation
…e-of-gas-with-i64-#1884 Optimize gas cost subtraction by using checked_sub for safer arithmet…
CodSpeed Performance ReportMerging #2062 will not alter performanceComparing Summary
|
The diff looks fine, but the PR is not related to the linked issue (#1884), and the motivations are incorrect. The generated code is identical: https://godbolt.org/z/83hTeP3vM |
Thanks for the feedback! The goal was to improve performance by reducing unnecessary branching. checked_sub removes the need for manual overflow handling, making the code cleaner and reducing conditional jumps. While the assembly may look similar, it's a safer and more efficient way to handle gas subtraction in u64. For #1884, I considered using i64 to simplify the out-of-gas check (< 0 instead of overflow handling). But since self.remaining is u64, switching would introduce underflow issues. If needed, I can change remaining to i64, but that would also require updates to functions like spent() and spent_sub_refunded() to handle negatives properly. Given that, checked_sub is the best balance of correctness and performance. Let me know what you think! |
As @DaniPopes pointed out, it is the same assembly, so only semantics are changed. Last your PR seemed more interesting: #2060
We don't have a branch here but |
I've already tried this here #1940 I did not measure any improvement as the branch still occurs in all callers to early return the gas overflow error regardless |
Ack, thanks for checking it! It looks like there are no gains here. Will close the original issue, and would rename this one to |
Co-authored-by: rakita <rakita@users.noreply.github.com>
No worries, i have made that committed suggested changes , if you want you can merge it ,if not its fine either way! I'll look into some other good first issue @DaniPopes @rakita |
Summary
overflowing_sub
withchecked_sub
for safer gas cost subtraction.overflowing_sub
required an explicit overflow check, whilechecked_sub
directly prevents underflow without extra branching.u64
arithmetic while maintaining performance.Why Not Use
i64
Directly?i64
allows for a simple< 0
check, usingi64
would require changingself.remaining
fromu64
, which is not currently feasible.i64
, it would simplify out-of-gas checks (remaining - cost < 0
), which is CPU-friendly.self.remaining
isu64
,checked_sub
is the best alternative for preventing underflow while keeping performance high.Pipeline Considerations
overflowing_sub
behavior.