Refine collect scope function and reduce system call overhead #21
Conversation
zhochi01
commented
Dec 13, 2024
zhochi01
commented
- Fix main thread counter, only reserve main thread cpu cycles event.
- Replace read syscall with PMEVCNTR2_EL0 to reduce extra overhead.
- Add PMU validation related check.
collectors/perf.cpp
Outdated
| mSet = mConfig.get("set", -1).asInt(); | ||
| mInherit = mConfig.get("inherit", 1).asInt(); | ||
| mInherit = mConfig.get("inherit", 0).asInt(); |
There was a problem hiding this comment.
Keep default value as 1, as most test cases, especially the ST tests, use "1" by default.
For perAPI, setting "inherit" in input json file.
There was a problem hiding this comment.
It has been changed in new commit.
collectors/perf.cpp
Outdated
| @@ -698,6 +617,7 @@ void PerfCollector::summarize() | |||
| bool event_context::init(std::vector<struct event> &events, int tid, int cpu) | |||
| { | |||
| struct counter grp; | |||
| grp.name = "CPUCycleCount"; | |||
There was a problem hiding this comment.
grp.name is overridden later by events.name.
There was a problem hiding this comment.
I have removed this in new commit.
test.cpp
Outdated
| @@ -348,16 +348,18 @@ class Test8 { | |||
| tmp *= rand(); | |||
| }; | |||
|
|
|||
| pid_t tid = syscall(SYS_gettid); | |||
|
|
|||
There was a problem hiding this comment.
in test8, Collection is created without enablePerapiPerf enabled. (see Line 322)
collect_scope_start/stop for per API performance does work?
There was a problem hiding this comment.
I have refactored the test case and have made it worked. But I removed all of background threads since we really don't want to capture any background threads cycles for per api. It's only designed for profiling main thread performance.
collectors/perf.cpp
Outdated
| { | ||
| t.eventCtx.collect_scope(now, func_id, false); | ||
| } | ||
| mReplayThreads[tid].eventCtx.collect_scope(func_id, false, get_pmu_bits()); |
There was a problem hiding this comment.
- consider multiple pmus
- mReplayThreads is vector, and tid is not the index of mReplayThreads.
There was a problem hiding this comment.
Thanks for your advice. But the per api new function only want to focus on the CPU cycles. And it will only read cycles event counter register. So I wonder if it is not necessary to support multi-events in this case?
collectors/perf.cpp
Outdated
| volatile uint64_t pmcr_el0; | ||
| asm volatile("mrs %0, PMCR_EL0" : "=r"(pmcr_el0)); | ||
| pmu_counter_bits = ((pmcr_el0 & 0x80) == 0x80 ? 64 : 32); | ||
| if (!mConfig.isMember("type")) exit(EXIT_FAILURE); |
There was a problem hiding this comment.
is there a "type" field in mConfig?
There was a problem hiding this comment.
Yes. I found this type is different for different platforms. And if the type is wrong, PMEVCNTR2_EL0 register will read nothing. So I wonder if it's possible to set by json file.
