-
Notifications
You must be signed in to change notification settings - Fork 161
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
feat(prof) I/O profiling preview #3083
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3083 +/- ##
============================================
- Coverage 74.86% 74.85% -0.02%
Complexity 2792 2792
============================================
Files 112 112
Lines 11046 11046
============================================
- Hits 8270 8268 -2
- Misses 2776 2778 +2
Flags with carried forward coverage won't be shown. Click here to find out more. see 1 file with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Benchmarks [ profiler ]Benchmark execution time: 2025-02-20 17:04:02 Comparing candidate commit c406db0 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 29 metrics, 7 unstable metrics. |
6244902
to
826265c
Compare
016d5c8
to
2b73c8e
Compare
ValueType::new("socket-read-size", "bytes"), | ||
ValueType::new("socket-write-size", "bytes"), | ||
ValueType::new("file-io-read-size", "bytes"), | ||
ValueType::new("file-io-write-size", "bytes"), |
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.
At this point, with that many sample types, I think this should look like:
let cpu_time = system_settings.profiling_experimental_cpu_time_enabled;
let alloc = system_settings.profiling_allocation_enabled;
#[cfg(feature = "timeline")]
let timeline = system_settings.profiling_timeline_enabled;
#[cfg(not(feature = "timeline"))]
let timeline = false;
#[cfg(feature = "exception_profiling")]
let exception = system_settings.profiling_allocation_enabled;
#[cfg(not(feature = "exception_profiling"))]
let exception = false;
#[cfg(feature = "io_profiling")]
let io = system_settings.profiling_io_enabled;
#[cfg(not(feature = "io_profiling"))]
let io = false;
let sample_types = vec![
(ValueType::new("sample", "count"), true, mem::offset_of!(SampleValues, interrupt_count)),
(ValueType::new("wall-time", "nanoseconds"), true, mem::offset_of!(SampleValues, wall_time)),
(ValueType::new("cpu-time", "nanoseconds"), cpu_time, mem::offset_of!(SampleValues, cpu_time)),
(ValueType::new("alloc-samples", "count"), allocation, mem::offset_of!(SampleValues, alloc_samples)),
(ValueType::new("alloc-size", "bytes"), allocation, mem::offset_of!(SampleValues, alloc_size)),
(ValueType::new("timeline", "nanoseconds"), timeline, mem::offset_of!(SampleValues, timeline)),
(ValueType::new("exception-samples", "count"), exception, mem::offset_of!(SampleValues, exception)),
(ValueType::new("socket-read-time", "nanoseconds"), io, mem::offset_of!(SampleValues, socket_read_time)),
(ValueType::new("socket-write-time", "nanoseconds"), io, mem::offset_of!(SampleValues, socket_write_time)),
(ValueType::new("file-io-read-time", "nanoseconds"), io, mem::offset_of!(SampleValues, file_read_time)),
(ValueType::new("file-io-write-time", "nanoseconds"), io, mem::offset_of!(SampleValues, file_write_time)),
(ValueType::new("socket-read-size", "bytes"), io, mem::offset_of!(SampleValues, socket_read_size)),
(ValueType::new("socket-write-size", "bytes"), io, mem::offset_of!(SampleValues, socket_write_size)),
(ValueType::new("file-io-read-size", "bytes"), io, mem::offset_of!(SampleValues, file_read_size)),
(ValueType::new("file-io-write-size", "bytes"), io, mem::offset_of!(SampleValues, file_write_size)),
];
// and create sample_types(_mask) with a small loop from the tuples as well as offsets for filter() function
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.
(Does not need to be done now, just a suggestion for next time at least)
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.
Doing this in a follow-up PR
853e15b
to
79a3394
Compare
1447603
to
64d29b9
Compare
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.
Looks great to me now, thanks!
Description
This adds I/O profiling preview to the PHP profiler on Linux machines (it is only compiled for Linux, but technically this is a ELF64 feature and could work on other OS that are using ELF64 too):
DD_PROFILING_EXPERIMENTAL_IO_ENABLED
/datadog.profiling.experimental_io_enabled
configuration (defaultfalse
)socket_read_time
socket_write_time
file_read_time
file_write_time
socket_read_size
socket_write_size
file_read_size
file_write_size
Known limitations:
mmap()
is not observedmmap()
support)Those things will be added/handled in future PRs, also because this feature is still experimental. The sampling distances are best guess and will likely be adjusted later.
https://datadoghq.atlassian.net/browse/PROF-11288
Reviewer checklist