There may or may not be a readable ZEND_TRACE_OP_INFO() #2942
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2942 +/- ##
============================================
- Coverage 82.10% 73.97% -8.13%
Complexity 2527 2527
============================================
Files 108 108
Lines 10360 10360
============================================
- Hits 8506 7664 -842
- Misses 1854 2696 +842
Flags with carried forward coverage won't be shown. Click here to find out more. see 8 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Benchmarks [ tracer ]Benchmark execution time: 2024-11-13 16:22:13 Comparing candidate commit 0950a08 in PR branch Found 4 performance improvements and 2 performance regressions! Performance is the same for 172 metrics, 0 unstable metrics. scenario:HookBench/benchWithoutHook
scenario:LogsInjectionBench/benchLogsInfoInjection-opcache
scenario:SamplingRuleMatchingBench/benchRegexMatching1
scenario:SamplingRuleMatchingBench/benchRegexMatching3-opcache
scenario:SamplingRuleMatchingBench/benchRegexMatching4-opcache
scenario:TraceSerializationBench/benchSerializeTrace
|
Maybe I don't understand the code well enough, but I don't think it should happen. We had crashes there though, so that should probably should mitigate them. Signed-off-by: Bob Weinand <bob.weinand@datadoghq.com>
bwoebi
force-pushed
the
bob/check-opcache-trace-readable
branch
from
d785ced to
0950a08
Compare
| if (write(dd_probe_pipes[1], ZEND_OP_TRACE_INFO(opline, offset), sizeof(zend_op_trace_info)) < 0) { | ||
| return; | ||
| } | ||
| read(dd_probe_pipes[0], dummy_buf, sizeof(zend_op_trace_info)); |
There was a problem hiding this comment.
I came across the mincore function; could it be a simpler alternative?
There was a problem hiding this comment.
I wish I could use it, but it requires glibc 2.19. Whereas we currently target 2.17.
There was a problem hiding this comment.
Something, something, drop CentOS 7... :D
There was a problem hiding this comment.
You can use syscall directly and reference the syscall numbers and parameters in this table.
There was a problem hiding this comment.
@cataphract I suppose that will work, but using write() has the advantage of not being linux specific either.
Signed-off-by: Bob Weinand <bob.weinand@datadoghq.com>
Maybe I don't understand the code well enough, but I don't think it should happen. We had crashes there though, so that should probably should mitigate them.