-
Notifications
You must be signed in to change notification settings - Fork 862
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
Use "unlikely" with Ensure #6648
Conversation
Since conditions used with `Ensure` are not expected to fire except in rare circumstances, we should always treat this as unlikely.
@jnidzwetzki, @svenklemm: please review this pull request.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6648 +/- ##
==========================================
+ Coverage 80.05% 80.09% +0.03%
==========================================
Files 190 190
Lines 37179 37133 -46
Branches 9450 9433 -17
==========================================
- Hits 29763 29740 -23
+ Misses 3001 2994 -7
+ Partials 4415 4399 -16 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
PR timescale#6648 introduces a branch prediction in the Ensure macro. However, the checked condition needs to be negated to be unlikely. Therefore, the check predicts the branch likelihood wrongly. In other words, reporting an error in Ensure should be the unlikely case, and the condition should be fulfilled in most calls. This commit fixes the branch prediction.
PR timescale#6648 introduces a branch prediction in the Ensure macro. However, the checked condition needs to be negated to be unlikely. Therefore, the check predicts the branch likelihood wrongly. In other words, reporting an error in Ensure should be the unlikely case, and the condition should be fulfilled in most calls. This commit fixes the branch prediction.
PR #6648 introduces a branch prediction in the Ensure macro. However, the checked condition needs to be negated to be unlikely. Therefore, the check predicts the branch likelihood wrongly. In other words, reporting an error in Ensure should be the unlikely case, and the condition should be fulfilled in most calls. This commit fixes the branch prediction.
Since conditions used with
Ensure
are not expected to fire except in rare circumstances, we should always treat this as unlikely.Disable-check: force-changelog-file