[Lldb-commits] [lldb] 44103c9 - [trace][intelpt] Remove code smell when printing the raw trace size
Walter Erquinigo via lldb-commits
lldb-commits at lists.llvm.org
Tue Apr 12 13:08:16 PDT 2022
Author: Walter Erquinigo
Date: 2022-04-12T13:08:03-07:00
New Revision: 44103c96fa6b00e7824319de1b10ce26781e3852
URL: https://github.com/llvm/llvm-project/commit/44103c96fa6b00e7824319de1b10ce26781e3852
DIFF: https://github.com/llvm/llvm-project/commit/44103c96fa6b00e7824319de1b10ce26781e3852.diff
LOG: [trace][intelpt] Remove code smell when printing the raw trace size
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
Added:
Modified:
lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
Removed:
################################################################################
diff --git a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
index d08c50e60cdb7..651af3ea5b0cb 100644
--- a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
@@ -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();
}
@@ -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.
diff --git a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
index c89abcbcf4391..3f28afc658b60 100644
--- a/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
+++ b/lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
@@ -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.
diff --git a/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp b/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
index 1d9e8ce991117..a56d761086541 100644
--- a/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
@@ -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)
diff --git a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
index 10a542652ddfd..7ee496dd4ba7f 100644
--- a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
+++ b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
@@ -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",
@@ -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);
@@ -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() {
diff --git a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
index ca855f1abccdb..f8f60da7f52ce 100644
--- a/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
+++ b/lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.h
@@ -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;
More information about the lldb-commits
mailing list