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

fix: speed up assert default details case #1503

Merged
merged 2 commits into from
Mar 1, 2023

Conversation

erights
Copy link
Contributor

@erights erights commented Mar 1, 2023

Pick the lowest hanging performance fruit.

In general we try to avoid the

assert(cond, X`...`);

style, and use instead

cond || Fail`...`;

However, sometimes we are confident enough that the failure can never happen that we do not bother with the details (the

Something`...`

part). This happens often when we're just doing the assertion to inform the type checker of something we believe to be true. For these cases, we often write

assert(cond);

Yes, the success case is slower than the || Fail pattern by a function call. But the success case should not be more slower than that. It was, due to a completely unnecessary function call within the default argument for assert's optional second parameter.

Also fixed: The optional third parameter also happened to be doing work that was unneeded in the success case.

@erights erights self-assigned this Mar 1, 2023
@erights erights force-pushed the markm-lowest-handing-performance-fruit branch from 93e5163 to edbab85 Compare March 1, 2023 03:04
@erights erights force-pushed the markm-lowest-handing-performance-fruit branch from edbab85 to a4ffe39 Compare March 1, 2023 03:15
@erights erights requested review from FUDCo and mhofman March 1, 2023 03:17
@erights erights force-pushed the markm-lowest-handing-performance-fruit branch from f31ba43 to 70589d3 Compare March 1, 2023 03:49
@erights erights enabled auto-merge (squash) March 1, 2023 03:56
@erights erights merged commit 5915813 into master Mar 1, 2023
@erights erights deleted the markm-lowest-handing-performance-fruit branch March 1, 2023 03:57
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