-
Notifications
You must be signed in to change notification settings - Fork 5.7k
[Performance fix] Make GetBlob assuming elements are cached #38336
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
Conversation
Thanks for your contribution! |
@pawelpiotrowicz , @tsocha , @Silv3S, @piotrekobiIntel, @zuzg Please review |
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
#define likely(expr) (expr) | ||
#define unlikely(expr) (expr) | ||
#else | ||
#define likely(expr) (__builtin_expect(!!(expr), 1)) |
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.
Why are using double !! ??
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.
__builtin_expect does accept long integer arguments. Comparison among boolean and int may raise a warning. So it was "double negated" so True is converted to 1, False to 0 and compared with long in of 1 or 0.
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
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
@Aganlengzi Please review and merge if correct |
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
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
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 types
Performance optimization
PR changes
Others
Describe
GetBlob is very often used function and majority of it is executed under critical section so it is good to optimize it as much as possible. In this PR we add assumption that objects in cache are there e.g. we instruct compiler to generate code layout in favour of situation that objects are cached. Performance improvement is x < 1% on mobilenetv1_int8 CLX