Skip to content

Commit

Permalink
[trace][intelpt] Remove code smell when printing the raw trace size
Browse files Browse the repository at this point in the history
Something ugly I did was to report the trace buffer size to the DecodedThread,
which is later used as part of the `dump info` command. Instead of doing that,
we can just directly ask the trace for the raw buffer and print its size.

I thought about not asking for the entire trace but instead just for its size,
but in this case, as our traces as not extremely big, I prefer to ask for the
entire trace, ensuring it could be fetched, and then print its size.

Differential Revision: https://reviews.llvm.org/D123358
  • Loading branch information
walter-erquinigo committed Apr 12, 2022
1 parent bdf3e7e commit 44103c9
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 34 deletions.
6 changes: 0 additions & 6 deletions lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
Expand Up @@ -35,10 +35,6 @@ void IntelPTError::log(llvm::raw_ostream &OS) const {
OS << "error: " << libipt_error_message;
}

Optional<size_t> DecodedThread::GetRawTraceSize() const {
return m_raw_trace_size;
}

size_t DecodedThread::GetInstructionsCount() const {
return m_instruction_ips.size();
}
Expand Down Expand Up @@ -178,8 +174,6 @@ DecodedThread::DecodedThread(ThreadSP thread_sp, Error &&error)
AppendError(std::move(error));
}

void DecodedThread::SetRawTraceSize(size_t size) { m_raw_trace_size = size; }

lldb::TraceCursorUP DecodedThread::GetCursor() {
// We insert a fake error signaling an empty trace if needed becasue the
// TraceCursor requires non-empty traces.
Expand Down
9 changes: 0 additions & 9 deletions lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
Expand Up @@ -175,15 +175,6 @@ class DecodedThread : public std::enable_shared_from_this<DecodedThread> {
/// Get a new cursor for the decoded thread.
lldb::TraceCursorUP GetCursor();

/// Set the size in bytes of the corresponding Intel PT raw trace.
void SetRawTraceSize(size_t size);

/// Get the size in bytes of the corresponding Intel PT raw trace.
///
/// \return
/// The size of the trace, or \b llvm::None if not available.
llvm::Optional<size_t> GetRawTraceSize() const;

/// Return the number of TSC decoding errors that happened. A TSC error
/// is not a fatal error and doesn't create gaps in the trace. Instead
/// we only keep track of them as a statistic.
Expand Down
2 changes: 0 additions & 2 deletions lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
Expand Up @@ -290,8 +290,6 @@ CreateInstructionDecoder(DecodedThread &decoded_thread,
void lldb_private::trace_intel_pt::DecodeTrace(DecodedThread &decoded_thread,
TraceIntelPT &trace_intel_pt,
ArrayRef<uint8_t> buffer) {
decoded_thread.SetRawTraceSize(buffer.size());

Expected<PtInsnDecoderUP> decoder_up =
CreateInstructionDecoder(decoded_thread, trace_intel_pt, buffer);
if (!decoder_up)
Expand Down
42 changes: 26 additions & 16 deletions lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
Expand Up @@ -106,20 +106,27 @@ lldb::TraceCursorUP TraceIntelPT::GetCursor(Thread &thread) {
}

void TraceIntelPT::DumpTraceInfo(Thread &thread, Stream &s, bool verbose) {
Optional<size_t> raw_size = GetRawTraceSize(thread);
lldb::tid_t tid = thread.GetID();
s.Format("\nthread #{0}: tid = {1}", thread.GetIndexID(), thread.GetID());
if (!raw_size) {
if (!IsTraced(tid)) {
s << ", not traced\n";
return;
}
s << "\n";

Expected<size_t> raw_size = GetRawTraceSize(thread);
if (!raw_size) {
s.Format(" {0}\n", toString(raw_size.takeError()));
return;
}

DecodedThreadSP decoded_trace_sp = Decode(thread);
size_t insn_len = decoded_trace_sp->GetInstructionsCount();
size_t mem_used = decoded_trace_sp->CalculateApproximateMemoryUsage();

s.Format(" Total number of instructions: {0}\n", insn_len);

s.PutCString("\n Memory usage:\n");
s << "\n Memory usage:\n";
s.Format(" Raw trace size: {0} KiB\n", *raw_size / 1024);
s.Format(
" Total approximate memory usage (excluding raw trace): {0:2} KiB\n",
Expand All @@ -129,15 +136,13 @@ void TraceIntelPT::DumpTraceInfo(Thread &thread, Stream &s, bool verbose) {
"{0:2} bytes\n",
(double)mem_used / insn_len);

s.PutCString("\n Timing:\n");
GetTimer()
.ForThread(thread.GetID())
.ForEachTimedTask(
[&](const std::string &name, std::chrono::milliseconds duration) {
s.Format(" {0}: {1:2}s\n", name, duration.count() / 1000.0);
});
s << "\n Timing:\n";
GetTimer().ForThread(tid).ForEachTimedTask(
[&](const std::string &name, std::chrono::milliseconds duration) {
s.Format(" {0}: {1:2}s\n", name, duration.count() / 1000.0);
});

s.PutCString("\n Errors:\n");
s << "\n Errors:\n";
const DecodedThread::LibiptErrors &tsc_errors =
decoded_trace_sp->GetTscErrors();
s.Format(" Number of TSC decoding errors: {0}\n", tsc_errors.total_count);
Expand All @@ -147,11 +152,16 @@ void TraceIntelPT::DumpTraceInfo(Thread &thread, Stream &s, bool verbose) {
}
}

Optional<size_t> TraceIntelPT::GetRawTraceSize(Thread &thread) {
if (IsTraced(thread.GetID()))
return Decode(thread)->GetRawTraceSize();
else
return None;
llvm::Expected<size_t> TraceIntelPT::GetRawTraceSize(Thread &thread) {
size_t size;
auto callback = [&](llvm::ArrayRef<uint8_t> data) {
size = data.size();
return Error::success();
};
if (Error err = OnThreadBufferRead(thread.GetID(), callback))
return std::move(err);

return size;
}

Expected<pt_cpu> TraceIntelPT::GetCPUInfoForLiveProcess() {
Expand Down
2 changes: 1 addition & 1 deletion lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
Expand Up @@ -73,7 +73,7 @@ class TraceIntelPT : public Trace {

void DumpTraceInfo(Thread &thread, Stream &s, bool verbose) override;

llvm::Optional<size_t> GetRawTraceSize(Thread &thread);
llvm::Expected<size_t> GetRawTraceSize(Thread &thread);

void DoRefreshLiveProcessState(
llvm::Expected<TraceGetStateResponse> state) override;
Expand Down

0 comments on commit 44103c9

Please sign in to comment.