-
Notifications
You must be signed in to change notification settings - Fork 5
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
Add logging to the library #359
Conversation
nsavoire
commented
Jan 11, 2024
•
edited
Loading
edited
- Setup logger in library
- Use DD_PROFILING_NATIVE_LIBRARY_LOG_MODE with fallback to DD_PROFILING_NATIVE_LOG_MODE to determine library log mode
- Use DD_PROFILING_NATIVE_LIBRARY_LOG_LEVEL with fallback to DD_PROFILING_NATIVE_LOG_LEVEL to determine library log level
- Add callback in logger to determine if log is allowed, this callback is used to disable logging when allocations are forbidden
fee1626
to
b55c53b
Compare
Benchmark results for collatzParameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 1 metrics, 0 unstable metrics. See unchanged results
|
Benchmark results for BadBoggleSolver_runParameters
SummaryFound 0 performance improvements and 0 performance regressions! Performance is the same for 1 metrics, 0 unstable metrics. See unchanged results
|
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
Should we delay the change to 0.16, or do you feel the change is harmless ?
86873df
to
91adb83
Compare
src/lib/allocation_tracker.cc
Outdated
if (_state.pid == 0) { | ||
_state.pid = getpid(); | ||
} | ||
if (tl_state.tid == 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.
maybe leaving a cassert is nice
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
a7eadd5
to
fbd0b92
Compare
#else | ||
// DDPROF_CHECK_FATAL must not be used inside profiling library | ||
# define DDPROF_CHECK_FATAL(condition, ...) \ | ||
static_assert( \ |
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.
Good thinking
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
fbd0b92
to
26c39e0
Compare
26c39e0
to
6ec12ac
Compare
* Setup logger in library * Use DD_PROFILING_NATIVE_LIBRARY_LOG_MODE with fallback to DD_PROFILING_NATIVE_LOG_MODE to determine library log mode * Use DD_PROFILING_NATIVE_LIBRARY_LOG_LEVEL with fallback to DD_PROFILING_NATIVE_LOG_LEVEL to determine library log level * Add callback in logger to determine if log is allowed, this callback is used to disable logging when allocations are forbidden
a0e82a7
to
f8c1438
Compare
Using arrays made the code incorrect when embedded data was not present, since their addresses and the resulting computed size would be non-NULL.
f8c1438
to
9790fe1
Compare
@r1viollet |
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