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; _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits