diff options
author | Yabin Cui <yabinc@google.com> | 2023-04-25 18:10:29 +0000 |
---|---|---|
committer | Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com> | 2023-04-25 18:10:29 +0000 |
commit | e7b09348289f0217b5292ada5c0ff6711246205e (patch) | |
tree | 6ed311f3105e82d0fd39d901c9a0bf8df8baab53 | |
parent | 808d063818dedeaa48a33e97ee46c8ef3bc9507a (diff) | |
parent | c87b4619908209cae2c2f7ca3efb4a1875e688d3 (diff) | |
download | extras-e7b09348289f0217b5292ada5c0ff6711246205e.tar.gz |
simpleperf: Remove stack and register fields for samples without am: c87b461990
Original change: https://googleplex-android-review.googlesource.com/c/platform/system/extras/+/22795620
Change-Id: I24e25c3ea4d4e7f100c51b331a19b97c141bc315
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
-rw-r--r-- | simpleperf/cmd_record.cpp | 11 | ||||
-rw-r--r-- | simpleperf/record.cpp | 39 | ||||
-rw-r--r-- | simpleperf/record_test.cpp | 31 |
3 files changed, 55 insertions, 26 deletions
diff --git a/simpleperf/cmd_record.cpp b/simpleperf/cmd_record.cpp index c9ae6591..e168305e 100644 --- a/simpleperf/cmd_record.cpp +++ b/simpleperf/cmd_record.cpp @@ -1727,9 +1727,11 @@ void RecordCommand::UpdateRecord(Record* record) { } bool RecordCommand::UnwindRecord(SampleRecord& r) { - if ((r.sample_type & PERF_SAMPLE_CALLCHAIN) && (r.sample_type & PERF_SAMPLE_REGS_USER) && - (r.regs_user_data.reg_mask != 0) && (r.sample_type & PERF_SAMPLE_STACK_USER) && - (r.GetValidStackSize() > 0)) { + if (!(r.sample_type & PERF_SAMPLE_CALLCHAIN) && (r.sample_type & PERF_SAMPLE_REGS_USER) && + (r.regs_user_data.reg_mask != 0) && (r.sample_type & PERF_SAMPLE_STACK_USER)) { + return true; + } + if (r.GetValidStackSize() > 0) { ThreadEntry* thread = thread_tree_.FindThreadOrNew(r.tid_data.pid, r.tid_data.tid); RegSet regs(r.regs_user_data.abi, r.regs_user_data.reg_mask, r.regs_user_data.regs); std::vector<uint64_t> ips; @@ -1758,6 +1760,9 @@ bool RecordCommand::UnwindRecord(SampleRecord& r) { CallChainJoiner::ORIGINAL_OFFLINE, ips, sps)) { return false; } + } else { + // For kernel samples, we still need to remove user stack and register fields. + r.ReplaceRegAndStackWithCallChain({}); } return true; } diff --git a/simpleperf/record.cpp b/simpleperf/record.cpp index c364b2fc..29e5d112 100644 --- a/simpleperf/record.cpp +++ b/simpleperf/record.cpp @@ -557,6 +557,8 @@ bool SampleRecord::Parse(const perf_event_attr& attr, char* p, char* end) { MoveFromBinaryFormat(regs_user_data.abi, p); if (regs_user_data.abi == 0) { regs_user_data.reg_mask = 0; + regs_user_data.reg_nr = 0; + regs_user_data.regs = nullptr; } else { regs_user_data.reg_mask = attr.sample_regs_user; size_t bit_nr = __builtin_popcountll(regs_user_data.reg_mask); @@ -580,7 +582,7 @@ bool SampleRecord::Parse(const perf_event_attr& attr, char* p, char* end) { } // TODO: Add parsing of other PERF_SAMPLE_*. if (UNLIKELY(p < end)) { - LOG(DEBUG) << "Record has " << end - p << " bytes left\n"; + LOG(DEBUG) << "Sample (" << time_data.time << ") has " << end - p << " bytes left"; } return true; } @@ -711,11 +713,15 @@ SampleRecord::SampleRecord(const perf_event_attr& attr, uint64_t id, uint64_t ip } void SampleRecord::ReplaceRegAndStackWithCallChain(const std::vector<uint64_t>& ips) { - uint32_t size_added_in_callchain = sizeof(uint64_t) * (ips.size() + 1); - uint32_t size_reduced_in_reg_stack = - (regs_user_data.reg_nr + 1) * sizeof(uint64_t) + stack_user_data.size + sizeof(uint64_t) * 2; - - uint32_t new_size = size() + size_added_in_callchain - size_reduced_in_reg_stack; + uint32_t add_size_in_callchain = ips.empty() ? 0 : sizeof(uint64_t) * (ips.size() + 1); + uint32_t reduce_size_in_reg = (regs_user_data.reg_nr + 1) * sizeof(uint64_t); + uint32_t reduce_size_in_stack = + stack_user_data.size == 0 ? sizeof(uint64_t) : (stack_user_data.size + 2 * sizeof(uint64_t)); + uint32_t reduce_size = reduce_size_in_reg + reduce_size_in_stack; + + uint32_t new_size = size() + add_size_in_callchain; + CHECK_GT(new_size, reduce_size); + new_size -= reduce_size; sample_type &= ~(PERF_SAMPLE_STACK_USER | PERF_SAMPLE_REGS_USER); BuildBinaryWithNewCallChain(new_size, ips); } @@ -818,16 +824,21 @@ void SampleRecord::BuildBinaryWithNewCallChain(uint32_t new_size, memcpy(p, &raw_data.size, sizeof(uint32_t)); } uint64_t* p64 = reinterpret_cast<uint64_t*>(p); - p64 -= ips.size(); - memcpy(p64, ips.data(), ips.size() * sizeof(uint64_t)); - *--p64 = PERF_CONTEXT_USER; - if (callchain_data.ip_nr > 0) { - p64 -= callchain_data.ip_nr; + if (!ips.empty()) { + p64 -= ips.size(); + memcpy(p64, ips.data(), ips.size() * sizeof(uint64_t)); + *--p64 = PERF_CONTEXT_USER; + } + p64 -= callchain_data.ip_nr; + if (p64 != callchain_data.ips) { memcpy(p64, callchain_data.ips, callchain_data.ip_nr * sizeof(uint64_t)); + callchain_data.ips = p64; + } + p64--; + if (!ips.empty()) { + callchain_data.ip_nr += 1 + ips.size(); + *p64 = callchain_data.ip_nr; } - callchain_data.ips = p64; - callchain_data.ip_nr += 1 + ips.size(); - *--p64 = callchain_data.ip_nr; CHECK_EQ(callchain_pos, static_cast<size_t>(reinterpret_cast<char*>(p64) - new_binary)) << "record time " << time_data.time; if (new_binary != binary_) { diff --git a/simpleperf/record_test.cpp b/simpleperf/record_test.cpp index 8d9d151a..20c9c18a 100644 --- a/simpleperf/record_test.cpp +++ b/simpleperf/record_test.cpp @@ -105,15 +105,28 @@ TEST_F(RecordTest, SampleRecord_exclude_kernel_callchain) { TEST_F(RecordTest, SampleRecord_ReplaceRegAndStackWithCallChain) { event_attr.sample_type |= PERF_SAMPLE_CALLCHAIN; - SampleRecord expected(event_attr, 0, 1, 2, 3, 4, 5, 6, {}, {1, PERF_CONTEXT_USER, 2, 3, 4, 5}, {}, - 0); - for (size_t stack_size : {8, 1024}) { - event_attr.sample_type |= PERF_SAMPLE_REGS_USER | PERF_SAMPLE_STACK_USER; - SampleRecord r(event_attr, 0, 1, 2, 3, 4, 5, 6, {}, {1}, std::vector<char>(stack_size), 10); - event_attr.sample_type &= ~(PERF_SAMPLE_REGS_USER | PERF_SAMPLE_STACK_USER); - r.ReplaceRegAndStackWithCallChain({2, 3, 4, 5}); - CheckRecordMatchBinary(r); - CheckRecordEqual(r, expected); + std::vector<std::vector<uint64_t>> user_ip_tests = { + {}, // no userspace ips, just remove stack and reg fields + {2}, // add one userspace ip, no need to allocate new binary + {2, 3, 4, 5, 6, 7, 8}, // add more userspace ips, may need to allocate new binary + }; + std::vector<uint64_t> stack_size_tests = {0, 8, 1024}; + + for (const auto& user_ips : user_ip_tests) { + std::vector<uint64_t> ips = {1}; + if (!user_ips.empty()) { + ips.push_back(PERF_CONTEXT_USER); + ips.insert(ips.end(), user_ips.begin(), user_ips.end()); + } + SampleRecord expected(event_attr, 0, 1, 2, 3, 4, 5, 6, {}, ips, {}, 0); + for (size_t stack_size : stack_size_tests) { + event_attr.sample_type |= PERF_SAMPLE_REGS_USER | PERF_SAMPLE_STACK_USER; + SampleRecord r(event_attr, 0, 1, 2, 3, 4, 5, 6, {}, {1}, std::vector<char>(stack_size), 10); + event_attr.sample_type &= ~(PERF_SAMPLE_REGS_USER | PERF_SAMPLE_STACK_USER); + r.ReplaceRegAndStackWithCallChain(user_ips); + CheckRecordMatchBinary(r); + CheckRecordEqual(r, expected); + } } } |