-
Notifications
You must be signed in to change notification settings - Fork 116
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
[DeviceASAN] Move validation layer before sanitizer layer #2251
base: main
Are you sure you want to change the base?
[DeviceASAN] Move validation layer before sanitizer layer #2251
Conversation
Avoid internal UR API calls from sanitizer layer being intercepted by validation layer for better performance.
Do we have some performance data here? |
Updated the git messages |
Great, a good improvement. |
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.
I'm not opposed, but I'm not sure I understand the concern about performance. The validation layer is disabled by default. Users can just chose to not enable it if they observe reduced performance.
Eh....Actually, it's because intel openmp offload runtime will enable validation layer by default in their ur plugin. BTW, the overhead comes mainly from 'backtrace_symbols'. Maybe we can also do same changes in validation layer as PR #2128 |
At some point we should move the backtrace implementation into common and have only one implementation. |
They should stop doing that. It hurts performance overall, not just when sanitizer is enabled. If they want to retain some lightweight validation, they should enable only parameter validation (which is reasonable to leave always enabled). |
Thanks. I'll suggest them not to enable full validation layer by default. |
Avoid internal UR API calls from sanitizer layer being intercepted by validation layer for better performance.
For miniqmc, the runtime can be reduced by -21.46% with this change.