[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction
This revision was automatically updated to reflect the committed changes.
Closed by commit rGca922a3559d7: [intelpt] Refactor timestamps out of
`IntelPTInstruction` (authored by zrthxn).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122603/new/
https://reviews.llvm.org/D122603
Files:
lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
lldb/test/API/commands/trace/TestTraceDumpInfo.py
lldb/test/API/commands/trace/TestTraceLoad.py
Index: lldb/test/API/commands/trace/TestTraceLoad.py
===
--- lldb/test/API/commands/trace/TestTraceLoad.py
+++ lldb/test/API/commands/trace/TestTraceLoad.py
@@ -38,8 +38,8 @@
thread #1: tid = 3842849
Raw trace size: 4 KiB
Total number of instructions: 21
- Total approximate memory usage: 5.31 KiB
- Average memory usage per instruction: 259 bytes'''])
+ Total approximate memory usage: 0.98 KiB
+ Average memory usage per instruction: 48.00 bytes'''])
def testLoadInvalidTraces(self):
src_dir = self.getSourceDir()
Index: lldb/test/API/commands/trace/TestTraceDumpInfo.py
===
--- lldb/test/API/commands/trace/TestTraceDumpInfo.py
+++ lldb/test/API/commands/trace/TestTraceDumpInfo.py
@@ -40,5 +40,5 @@
thread #1: tid = 3842849
Raw trace size: 4 KiB
Total number of instructions: 21
- Total approximate memory usage: 5.31 KiB
- Average memory usage per instruction: 259 bytes'''])
+ Total approximate memory usage: 0.98 KiB
+ Average memory usage per instruction: 48.00 bytes'''])
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
@@ -119,8 +119,9 @@
s.Printf(" Total number of instructions: %zu\n", insn_len);
s.Printf(" Total approximate memory usage: %0.2lf KiB\n",
(double)mem_used / 1024);
- s.Printf(" Average memory usage per instruction: %zu bytes\n",
- mem_used / insn_len);
+ if (insn_len != 0)
+s.Printf(" Average memory usage per instruction: %0.2lf bytes\n",
+ (double)mem_used / insn_len);
return;
}
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
@@ -42,6 +42,8 @@
DecodedThreadSP m_decoded_thread_sp;
/// Internal instruction index currently pointing at.
size_t m_pos;
+ /// Tsc range covering the current instruction.
+ llvm::Optional m_tsc_range;
};
} // namespace trace_intel_pt
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
@@ -23,6 +23,7 @@
assert(!m_decoded_thread_sp->GetInstructions().empty() &&
"a trace should have at least one instruction or error");
m_pos = m_decoded_thread_sp->GetInstructions().size() - 1;
+ m_tsc_range = m_decoded_thread_sp->CalculateTscRange(m_pos);
}
size_t TraceCursorIntelPT::GetInternalInstructionSize() {
@@ -40,6 +41,10 @@
while (canMoveOne()) {
m_pos += IsForwards() ? 1 : -1;
+
+if (m_tsc_range && !m_tsc_range->InRange(m_pos))
+ m_tsc_range = IsForwards() ? m_tsc_range->Next() : m_tsc_range->Prev();
+
if (!m_ignore_errors && IsError())
return true;
if (GetInstructionControlFlowType() & m_granularity)
@@ -58,23 +63,29 @@
return std::min(std::max((int64_t)0, raw_pos), last_index);
};
- switch (origin) {
- case TraceCursor::SeekType::Set:
-m_pos = fitPosToBounds(offset);
-return m_pos;
- case TraceCursor::SeekType::End:
-m_pos = fitPosToBounds(offset + last_index);
-return last_index - m_pos;
- case TraceCursor::SeekType::Current:
-int64_t new_pos = fitPosToBounds(offset + m_pos);
-int64_t dist = m_pos - new_pos;
-m_pos = new_pos;
-return std::abs(dist);
- }
+ auto FindDistanceAndSetPos = [&]() -> int64_t {
+switch (origin) {
+case TraceCursor::SeekType::Set:
+ m_pos = fitPosToBounds(offset);
+ return m_pos;
+case TraceCursor::SeekType::End:
+ m_pos = fitPosToBounds(offset + last_index);
+ return last_index - m_pos;
+case TraceCursor::SeekType::Current:
+ int64_t new_pos = fitPosToBounds(offset + m_pos);
+ int64_t dist = m_pos - new_pos;
+ m_pos = new_pos;
+ return std::abs(dist);
+}
+ };
+
+ int6
[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction
zrthxn updated this revision to Diff 419656.
zrthxn edited the summary of this revision.
zrthxn added a comment.
The difference in memory usage is appreciable with large number of
instructions, as shown below
# Before (with current metrics, total memory does not include raw trace size)
Raw trace size: 2048 KiB
Total number of instructions: 94
Total approximate memory usage: 56143.10 KiB
Average memory usage per instruction: 63.87 bytes
# After
Raw trace size: 2048 KiB
Total number of instructions: 94
Total approximate memory usage: 42187.69 KiB
Average memory usage per instruction: 48.00 bytes
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122603/new/
https://reviews.llvm.org/D122603
Files:
lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
lldb/test/API/commands/trace/TestTraceDumpInfo.py
lldb/test/API/commands/trace/TestTraceLoad.py
Index: lldb/test/API/commands/trace/TestTraceLoad.py
===
--- lldb/test/API/commands/trace/TestTraceLoad.py
+++ lldb/test/API/commands/trace/TestTraceLoad.py
@@ -38,8 +38,8 @@
thread #1: tid = 3842849
Raw trace size: 4 KiB
Total number of instructions: 21
- Total approximate memory usage: 5.31 KiB
- Average memory usage per instruction: 259 bytes'''])
+ Total approximate memory usage: 0.98 KiB
+ Average memory usage per instruction: 48.00 bytes'''])
def testLoadInvalidTraces(self):
src_dir = self.getSourceDir()
Index: lldb/test/API/commands/trace/TestTraceDumpInfo.py
===
--- lldb/test/API/commands/trace/TestTraceDumpInfo.py
+++ lldb/test/API/commands/trace/TestTraceDumpInfo.py
@@ -40,5 +40,5 @@
thread #1: tid = 3842849
Raw trace size: 4 KiB
Total number of instructions: 21
- Total approximate memory usage: 5.31 KiB
- Average memory usage per instruction: 259 bytes'''])
+ Total approximate memory usage: 0.98 KiB
+ Average memory usage per instruction: 48.00 bytes'''])
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
@@ -119,8 +119,9 @@
s.Printf(" Total number of instructions: %zu\n", insn_len);
s.Printf(" Total approximate memory usage: %0.2lf KiB\n",
(double)mem_used / 1024);
- s.Printf(" Average memory usage per instruction: %zu bytes\n",
- mem_used / insn_len);
+ if (insn_len != 0)
+s.Printf(" Average memory usage per instruction: %0.2lf bytes\n",
+ (double)mem_used / insn_len);
return;
}
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
@@ -42,6 +42,8 @@
DecodedThreadSP m_decoded_thread_sp;
/// Internal instruction index currently pointing at.
size_t m_pos;
+ /// Tsc range covering the current instruction.
+ llvm::Optional m_tsc_range;
};
} // namespace trace_intel_pt
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
@@ -23,6 +23,7 @@
assert(!m_decoded_thread_sp->GetInstructions().empty() &&
"a trace should have at least one instruction or error");
m_pos = m_decoded_thread_sp->GetInstructions().size() - 1;
+ m_tsc_range = m_decoded_thread_sp->CalculateTscRange(m_pos);
}
size_t TraceCursorIntelPT::GetInternalInstructionSize() {
@@ -40,6 +41,10 @@
while (canMoveOne()) {
m_pos += IsForwards() ? 1 : -1;
+
+if (m_tsc_range && !m_tsc_range->InRange(m_pos))
+ m_tsc_range = IsForwards() ? m_tsc_range->Next() : m_tsc_range->Prev();
+
if (!m_ignore_errors && IsError())
return true;
if (GetInstructionControlFlowType() & m_granularity)
@@ -58,23 +63,29 @@
return std::min(std::max((int64_t)0, raw_pos), last_index);
};
- switch (origin) {
- case TraceCursor::SeekType::Set:
-m_pos = fitPosToBounds(offset);
-return m_pos;
- case TraceCursor::SeekType::End:
-m_pos = fitPosToBounds(offset + last_index);
-return last_index - m_pos;
- case TraceCursor::SeekType::Current:
-int64_t new_pos = fitPosToBounds(offset + m_pos);
-int64_t dist = m_pos - new_pos;
-m_pos = new_pos;
-return std::abs(dist);
- }
+ auto FindDistanceAndSetPos = [&]() -> int64_t
[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction
wallace added a comment. Don't forget to update the description of this diff and of the commit before pushing (you need to do both). Include the avg instruction size for a trace of at least 10k instructions as well :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122603/new/ https://reviews.llvm.org/D122603 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction
wallace accepted this revision. wallace added a comment. This revision is now accepted and ready to land. lgtm Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122603/new/ https://reviews.llvm.org/D122603 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction
zrthxn added inline comments. Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:145 + private: +friend class DecodedThread; + wallace wrote: > jj10306 wrote: > > nit: No need to friend the enclosing class since C++11 - > > https://en.cppreference.com/w/cpp/language/nested_types > TIL! We need the friend because we are using a private constructor from outside, in DecodedThread::CalculateTscRange and a couple other places. The idea is to let only DecodedThread create TscRange. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122603/new/ https://reviews.llvm.org/D122603 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction
zrthxn updated this revision to Diff 419619.
zrthxn marked 3 inline comments as done.
zrthxn added a comment.
Dont use auto for simple types
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122603/new/
https://reviews.llvm.org/D122603
Files:
lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
lldb/test/API/commands/trace/TestTraceDumpInfo.py
lldb/test/API/commands/trace/TestTraceLoad.py
Index: lldb/test/API/commands/trace/TestTraceLoad.py
===
--- lldb/test/API/commands/trace/TestTraceLoad.py
+++ lldb/test/API/commands/trace/TestTraceLoad.py
@@ -38,8 +38,8 @@
thread #1: tid = 3842849
Raw trace size: 4 KiB
Total number of instructions: 21
- Total approximate memory usage: 5.31 KiB
- Average memory usage per instruction: 259 bytes'''])
+ Total approximate memory usage: 0.98 KiB
+ Average memory usage per instruction: 48.00 bytes'''])
def testLoadInvalidTraces(self):
src_dir = self.getSourceDir()
Index: lldb/test/API/commands/trace/TestTraceDumpInfo.py
===
--- lldb/test/API/commands/trace/TestTraceDumpInfo.py
+++ lldb/test/API/commands/trace/TestTraceDumpInfo.py
@@ -40,5 +40,5 @@
thread #1: tid = 3842849
Raw trace size: 4 KiB
Total number of instructions: 21
- Total approximate memory usage: 5.31 KiB
- Average memory usage per instruction: 259 bytes'''])
+ Total approximate memory usage: 0.98 KiB
+ Average memory usage per instruction: 48.00 bytes'''])
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
@@ -119,8 +119,9 @@
s.Printf(" Total number of instructions: %zu\n", insn_len);
s.Printf(" Total approximate memory usage: %0.2lf KiB\n",
(double)mem_used / 1024);
- s.Printf(" Average memory usage per instruction: %zu bytes\n",
- mem_used / insn_len);
+ if (insn_len != 0)
+s.Printf(" Average memory usage per instruction: %0.2lf bytes\n",
+ (double)mem_used / insn_len);
return;
}
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
@@ -42,6 +42,8 @@
DecodedThreadSP m_decoded_thread_sp;
/// Internal instruction index currently pointing at.
size_t m_pos;
+ /// Tsc range covering the current instruction.
+ llvm::Optional m_tsc_range;
};
} // namespace trace_intel_pt
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
@@ -23,6 +23,7 @@
assert(!m_decoded_thread_sp->GetInstructions().empty() &&
"a trace should have at least one instruction or error");
m_pos = m_decoded_thread_sp->GetInstructions().size() - 1;
+ m_tsc_range = m_decoded_thread_sp->CalculateTscRange(m_pos);
}
size_t TraceCursorIntelPT::GetInternalInstructionSize() {
@@ -40,6 +41,10 @@
while (canMoveOne()) {
m_pos += IsForwards() ? 1 : -1;
+
+if (m_tsc_range && !m_tsc_range->InRange(m_pos))
+ m_tsc_range = IsForwards() ? m_tsc_range->Next() : m_tsc_range->Prev();
+
if (!m_ignore_errors && IsError())
return true;
if (GetInstructionControlFlowType() & m_granularity)
@@ -58,23 +63,29 @@
return std::min(std::max((int64_t)0, raw_pos), last_index);
};
- switch (origin) {
- case TraceCursor::SeekType::Set:
-m_pos = fitPosToBounds(offset);
-return m_pos;
- case TraceCursor::SeekType::End:
-m_pos = fitPosToBounds(offset + last_index);
-return last_index - m_pos;
- case TraceCursor::SeekType::Current:
-int64_t new_pos = fitPosToBounds(offset + m_pos);
-int64_t dist = m_pos - new_pos;
-m_pos = new_pos;
-return std::abs(dist);
- }
+ auto FindDistanceAndSetPos = [&]() -> int64_t {
+switch (origin) {
+case TraceCursor::SeekType::Set:
+ m_pos = fitPosToBounds(offset);
+ return m_pos;
+case TraceCursor::SeekType::End:
+ m_pos = fitPosToBounds(offset + last_index);
+ return last_index - m_pos;
+case TraceCursor::SeekType::Current:
+ int64_t new_pos = fitPosToBounds(offset + m_pos);
+ int64_t dist = m_pos - new_pos;
+ m_pos = new_pos;
+ return std::abs(dist);
+}
+ };
+
+ int64_t dist = FindDistanceAndSetPos();
+ m_tsc_
[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction
wallace added inline comments. Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:145 + private: +friend class DecodedThread; + jj10306 wrote: > nit: No need to friend the enclosing class since C++11 - > https://en.cppreference.com/w/cpp/language/nested_types TIL! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122603/new/ https://reviews.llvm.org/D122603 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction
jj10306 added inline comments. Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:145 + private: +friend class DecodedThread; + nit: No need to friend the enclosing class since C++11 - https://en.cppreference.com/w/cpp/language/nested_types Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122603/new/ https://reviews.llvm.org/D122603 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction
wallace added a comment. one last nit and good to go Comment at: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp:82 + + auto dist = FindDistanceAndSetPos(); + m_tsc_range = m_decoded_thread_sp->CalculateTscRange(m_pos); don't use auto for simple types Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122603/new/ https://reviews.llvm.org/D122603 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction
zrthxn added inline comments.
Comment at: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp:67-82
switch (origin) {
case TraceCursor::SeekType::Set:
m_pos = fitPosToBounds(offset);
+m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
return m_pos;
case TraceCursor::SeekType::End:
m_pos = fitPosToBounds(offset + last_index);
wallace wrote:
> we can simplify this so that we only invoke CalculateTscRange once
This is incorrect The converted code always returns 0. I've refactored it to
have CalculateTscRange once but its a side-effect-y function and will need some
future attention.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122603/new/
https://reviews.llvm.org/D122603
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction
zrthxn updated this revision to Diff 419560.
zrthxn marked 12 inline comments as done.
zrthxn added a comment.
Incorporate feedback and update tests
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122603/new/
https://reviews.llvm.org/D122603
Files:
lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
lldb/test/API/commands/trace/TestTraceDumpInfo.py
lldb/test/API/commands/trace/TestTraceLoad.py
Index: lldb/test/API/commands/trace/TestTraceLoad.py
===
--- lldb/test/API/commands/trace/TestTraceLoad.py
+++ lldb/test/API/commands/trace/TestTraceLoad.py
@@ -38,8 +38,8 @@
thread #1: tid = 3842849
Raw trace size: 4 KiB
Total number of instructions: 21
- Total approximate memory usage: 5.31 KiB
- Average memory usage per instruction: 259 bytes'''])
+ Total approximate memory usage: 0.98 KiB
+ Average memory usage per instruction: 48.00 bytes'''])
def testLoadInvalidTraces(self):
src_dir = self.getSourceDir()
Index: lldb/test/API/commands/trace/TestTraceDumpInfo.py
===
--- lldb/test/API/commands/trace/TestTraceDumpInfo.py
+++ lldb/test/API/commands/trace/TestTraceDumpInfo.py
@@ -40,5 +40,5 @@
thread #1: tid = 3842849
Raw trace size: 4 KiB
Total number of instructions: 21
- Total approximate memory usage: 5.31 KiB
- Average memory usage per instruction: 259 bytes'''])
+ Total approximate memory usage: 0.98 KiB
+ Average memory usage per instruction: 48.00 bytes'''])
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
@@ -119,8 +119,9 @@
s.Printf(" Total number of instructions: %zu\n", insn_len);
s.Printf(" Total approximate memory usage: %0.2lf KiB\n",
(double)mem_used / 1024);
- s.Printf(" Average memory usage per instruction: %zu bytes\n",
- mem_used / insn_len);
+ if (insn_len != 0)
+s.Printf(" Average memory usage per instruction: %0.2lf bytes\n",
+ (double)mem_used / insn_len);
return;
}
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
@@ -42,6 +42,8 @@
DecodedThreadSP m_decoded_thread_sp;
/// Internal instruction index currently pointing at.
size_t m_pos;
+ /// Tsc range covering the current instruction.
+ llvm::Optional m_tsc_range;
};
} // namespace trace_intel_pt
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
@@ -23,6 +23,7 @@
assert(!m_decoded_thread_sp->GetInstructions().empty() &&
"a trace should have at least one instruction or error");
m_pos = m_decoded_thread_sp->GetInstructions().size() - 1;
+ m_tsc_range = m_decoded_thread_sp->CalculateTscRange(m_pos);
}
size_t TraceCursorIntelPT::GetInternalInstructionSize() {
@@ -40,6 +41,10 @@
while (canMoveOne()) {
m_pos += IsForwards() ? 1 : -1;
+
+if (m_tsc_range && !m_tsc_range->InRange(m_pos))
+ m_tsc_range = IsForwards() ? m_tsc_range->Next() : m_tsc_range->Prev();
+
if (!m_ignore_errors && IsError())
return true;
if (GetInstructionControlFlowType() & m_granularity)
@@ -58,23 +63,29 @@
return std::min(std::max((int64_t)0, raw_pos), last_index);
};
- switch (origin) {
- case TraceCursor::SeekType::Set:
-m_pos = fitPosToBounds(offset);
-return m_pos;
- case TraceCursor::SeekType::End:
-m_pos = fitPosToBounds(offset + last_index);
-return last_index - m_pos;
- case TraceCursor::SeekType::Current:
-int64_t new_pos = fitPosToBounds(offset + m_pos);
-int64_t dist = m_pos - new_pos;
-m_pos = new_pos;
-return std::abs(dist);
- }
+ auto FindDistanceAndSetPos = [&]() -> int64_t {
+switch (origin) {
+case TraceCursor::SeekType::Set:
+ m_pos = fitPosToBounds(offset);
+ return m_pos;
+case TraceCursor::SeekType::End:
+ m_pos = fitPosToBounds(offset + last_index);
+ return last_index - m_pos;
+case TraceCursor::SeekType::Current:
+ int64_t new_pos = fitPosToBounds(offset + m_pos);
+ int64_t dist = m_pos - new_pos;
+ m_pos = new_pos;
+ return std::abs(dist);
+}
+ };
+
+ auto dist = FindDistanceAndSetPos();
+ m_
[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction
wallace requested changes to this revision.
wallace added a comment.
This revision now requires changes to proceed.
almost there! Mostly cosmetic changes needed
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:94-98
+ m_instructions.emplace_back(insn);
+ if (!m_last_tsc || *m_last_tsc != tsc) {
+m_instruction_timestamps.emplace(m_instructions.size() - 1, tsc);
+m_last_tsc = tsc;
+ }
We need to handle a special case. It might happen that the first instruction is
an error, which won't have a TSC, and the second instruction is an actual
instruction, and from that point on you'll always have TSCs. For this case, we
can assume that the TSC of the error instruction at the beginning of the trace
is the same as the first valid TSC.
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:112-113
+DecodedThread::CalculateTscRange(size_t insn_index) const {
+ if (m_instruction_timestamps.empty())
+return None;
+
delete these two lines. The rest of the code will work well without it
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:156-158
+DecodedThread::TscRange::TscRange(std::map::const_iterator
it,
+ const DecodedThread *decoded_thread)
+: m_it(it), m_decoded_thread(decoded_thread) {
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:104
// When adding new members to this class, make sure to update
- // IntelPTInstruction::GetNonErrorMemoryUsage() if needed.
+ // IntelPTInstruction::GetMemoryUsage() if needed.
pt_insn m_pt_insn;
+1
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:140
+/// Get the smallest instruction index that has this TSC.
+size_t GetStart() const;
+/// Get the largest instruction index that has this TSC.
Let's be more verbose with the names to improve readability
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:142
+/// Get the largest instruction index that has this TSC.
+size_t GetEnd() const;
+
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:148
+TscRange(std::map::const_iterator it,
+ const DecodedThread *decoded_thread);
+
let's receive a reference here and then convert it to pointer, so that we
minimize the number of places with pointers. Also, if later we decide to use a
shared_ptr instead of a pointer, we can do it inside of the constructor without
changing this line
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:150
+
+/// The current range
+std::map::const_iterator m_it;
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:181-182
+ /// Construct the TSC range that covers the given instruction index.
+ /// This operation is O(logn) and should be used sparingly.
+ llvm::Optional CalculateTscRange(size_t insn_index) const;
Comment at: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp:67-82
switch (origin) {
case TraceCursor::SeekType::Set:
m_pos = fitPosToBounds(offset);
+m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
return m_pos;
case TraceCursor::SeekType::End:
m_pos = fitPosToBounds(offset + last_index);
we can simplify this so that we only invoke CalculateTscRange once
Comment at: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h:46
+ /// Tsc range covering the current instruction.
+ llvm::Optional m_current_tsc;
};
rename it to `m_tsc_range`. The word current is very redundant in this case
Comment at: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp:123-124
+ if (insn_len != 0)
+s.Printf(" Average memory usage per instruction: %zu bytes\n",
+ mem_used / insn_len);
return;
Use doubles, as the average might not be a whole number
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122603/new/
https://reviews.llvm.org/D122603
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction
zrthxn updated this revision to Diff 419519.
zrthxn added a comment.
Updated tests according to new memory usage calculation
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122603/new/
https://reviews.llvm.org/D122603
Files:
lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
lldb/test/API/commands/trace/TestTraceDumpInfo.py
lldb/test/API/commands/trace/TestTraceLoad.py
Index: lldb/test/API/commands/trace/TestTraceLoad.py
===
--- lldb/test/API/commands/trace/TestTraceLoad.py
+++ lldb/test/API/commands/trace/TestTraceLoad.py
@@ -38,8 +38,8 @@
thread #1: tid = 3842849
Raw trace size: 4 KiB
Total number of instructions: 21
- Total approximate memory usage: 5.31 KiB
- Average memory usage per instruction: 259 bytes'''])
+ Total approximate memory usage: 0.98 KiB
+ Average memory usage per instruction: 48 bytes'''])
def testLoadInvalidTraces(self):
src_dir = self.getSourceDir()
Index: lldb/test/API/commands/trace/TestTraceDumpInfo.py
===
--- lldb/test/API/commands/trace/TestTraceDumpInfo.py
+++ lldb/test/API/commands/trace/TestTraceDumpInfo.py
@@ -40,5 +40,5 @@
thread #1: tid = 3842849
Raw trace size: 4 KiB
Total number of instructions: 21
- Total approximate memory usage: 5.31 KiB
- Average memory usage per instruction: 259 bytes'''])
+ Total approximate memory usage: 0.98 KiB
+ Average memory usage per instruction: 48 bytes'''])
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
@@ -119,8 +119,9 @@
s.Printf(" Total number of instructions: %zu\n", insn_len);
s.Printf(" Total approximate memory usage: %0.2lf KiB\n",
(double)mem_used / 1024);
- s.Printf(" Average memory usage per instruction: %zu bytes\n",
- mem_used / insn_len);
+ if (insn_len != 0)
+s.Printf(" Average memory usage per instruction: %zu bytes\n",
+ mem_used / insn_len);
return;
}
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
@@ -42,6 +42,8 @@
DecodedThreadSP m_decoded_thread_sp;
/// Internal instruction index currently pointing at.
size_t m_pos;
+ /// Tsc range covering the current instruction.
+ llvm::Optional m_current_tsc;
};
} // namespace trace_intel_pt
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
@@ -23,6 +23,7 @@
assert(!m_decoded_thread_sp->GetInstructions().empty() &&
"a trace should have at least one instruction or error");
m_pos = m_decoded_thread_sp->GetInstructions().size() - 1;
+ m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
}
size_t TraceCursorIntelPT::GetInternalInstructionSize() {
@@ -40,6 +41,11 @@
while (canMoveOne()) {
m_pos += IsForwards() ? 1 : -1;
+
+if (m_current_tsc && !m_current_tsc->InRange(m_pos))
+ m_current_tsc =
+ IsForwards() ? m_current_tsc->Next() : m_current_tsc->Prev();
+
if (!m_ignore_errors && IsError())
return true;
if (GetInstructionControlFlowType() & m_granularity)
@@ -61,20 +67,23 @@
switch (origin) {
case TraceCursor::SeekType::Set:
m_pos = fitPosToBounds(offset);
+m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
return m_pos;
case TraceCursor::SeekType::End:
m_pos = fitPosToBounds(offset + last_index);
+m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
return last_index - m_pos;
case TraceCursor::SeekType::Current:
int64_t new_pos = fitPosToBounds(offset + m_pos);
int64_t dist = m_pos - new_pos;
m_pos = new_pos;
+m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
return std::abs(dist);
}
}
bool TraceCursorIntelPT::IsError() {
- return m_decoded_thread_sp->GetInstructions()[m_pos].IsError();
+ return m_decoded_thread_sp->IsInstructionAnError(m_pos);
}
const char *TraceCursorIntelPT::GetError() {
@@ -85,10 +94,14 @@
return m_decoded_thread_sp->GetInstructions()[m_pos].GetLoadAddress();
}
-Optional TraceCursorIntelPT::GetCounter(lldb::TraceCounter counter_type) {
+Optional
+TraceCursorIntelPT::GetCou
[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction
zrthxn updated this revision to Diff 419517.
zrthxn marked an inline comment as done.
zrthxn added a comment.
Fixed issue with TSC becoming invalid midway through trace
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122603/new/
https://reviews.llvm.org/D122603
Files:
lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
@@ -119,8 +119,9 @@
s.Printf(" Total number of instructions: %zu\n", insn_len);
s.Printf(" Total approximate memory usage: %0.2lf KiB\n",
(double)mem_used / 1024);
- s.Printf(" Average memory usage per instruction: %zu bytes\n",
- mem_used / insn_len);
+ if (insn_len != 0)
+s.Printf(" Average memory usage per instruction: %zu bytes\n",
+ mem_used / insn_len);
return;
}
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
@@ -42,6 +42,8 @@
DecodedThreadSP m_decoded_thread_sp;
/// Internal instruction index currently pointing at.
size_t m_pos;
+ /// Tsc range covering the current instruction.
+ llvm::Optional m_current_tsc;
};
} // namespace trace_intel_pt
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
@@ -23,6 +23,7 @@
assert(!m_decoded_thread_sp->GetInstructions().empty() &&
"a trace should have at least one instruction or error");
m_pos = m_decoded_thread_sp->GetInstructions().size() - 1;
+ m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
}
size_t TraceCursorIntelPT::GetInternalInstructionSize() {
@@ -40,6 +41,11 @@
while (canMoveOne()) {
m_pos += IsForwards() ? 1 : -1;
+
+if (m_current_tsc && !m_current_tsc->InRange(m_pos))
+ m_current_tsc =
+ IsForwards() ? m_current_tsc->Next() : m_current_tsc->Prev();
+
if (!m_ignore_errors && IsError())
return true;
if (GetInstructionControlFlowType() & m_granularity)
@@ -61,20 +67,23 @@
switch (origin) {
case TraceCursor::SeekType::Set:
m_pos = fitPosToBounds(offset);
+m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
return m_pos;
case TraceCursor::SeekType::End:
m_pos = fitPosToBounds(offset + last_index);
+m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
return last_index - m_pos;
case TraceCursor::SeekType::Current:
int64_t new_pos = fitPosToBounds(offset + m_pos);
int64_t dist = m_pos - new_pos;
m_pos = new_pos;
+m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
return std::abs(dist);
}
}
bool TraceCursorIntelPT::IsError() {
- return m_decoded_thread_sp->GetInstructions()[m_pos].IsError();
+ return m_decoded_thread_sp->IsInstructionAnError(m_pos);
}
const char *TraceCursorIntelPT::GetError() {
@@ -85,10 +94,14 @@
return m_decoded_thread_sp->GetInstructions()[m_pos].GetLoadAddress();
}
-Optional TraceCursorIntelPT::GetCounter(lldb::TraceCounter counter_type) {
+Optional
+TraceCursorIntelPT::GetCounter(lldb::TraceCounter counter_type) {
switch (counter_type) {
-case lldb::eTraceCounterTSC:
- return m_decoded_thread_sp->GetInstructions()[m_pos].GetTimestampCounter();
+ case lldb::eTraceCounterTSC:
+if (m_current_tsc)
+ return m_current_tsc->GetTsc();
+else
+ return llvm::None;
}
}
Index: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
===
--- lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
+++ lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
@@ -62,9 +62,6 @@
/// As mentioned, any gap is represented as an error in this class.
class IntelPTInstruction {
public:
- IntelPTInstruction(const pt_insn &pt_insn, uint64_t timestamp)
- : m_pt_insn(pt_insn), m_timestamp(timestamp), m_is_error(false) {}
-
IntelPTInstruction(const pt_insn &pt_insn)
: m_pt_insn(pt_insn), m_is_error(false) {}
@@ -85,13 +82,6 @@
/// Get the size in bytes of an instance of this class
static size_t GetMemoryUsage();
- /// Get the timestamp associated with the current instruction. The timestamp
- /// is similar to what a rdtsc instruction would return.
- ///
- /// \ret
[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction
zrthxn updated this revision to Diff 419348.
zrthxn added a comment.
Change tsc check anyway
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122603/new/
https://reviews.llvm.org/D122603
Files:
lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
@@ -119,8 +119,9 @@
s.Printf(" Total number of instructions: %zu\n", insn_len);
s.Printf(" Total approximate memory usage: %0.2lf KiB\n",
(double)mem_used / 1024);
- s.Printf(" Average memory usage per instruction: %zu bytes\n",
- mem_used / insn_len);
+ if (insn_len != 0)
+s.Printf(" Average memory usage per instruction: %zu bytes\n",
+ mem_used / insn_len);
return;
}
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
@@ -42,6 +42,8 @@
DecodedThreadSP m_decoded_thread_sp;
/// Internal instruction index currently pointing at.
size_t m_pos;
+ /// Tsc range covering the current instruction.
+ llvm::Optional m_current_tsc;
};
} // namespace trace_intel_pt
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
@@ -23,6 +23,7 @@
assert(!m_decoded_thread_sp->GetInstructions().empty() &&
"a trace should have at least one instruction or error");
m_pos = m_decoded_thread_sp->GetInstructions().size() - 1;
+ m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
}
size_t TraceCursorIntelPT::GetInternalInstructionSize() {
@@ -40,6 +41,13 @@
while (canMoveOne()) {
m_pos += IsForwards() ? 1 : -1;
+
+if (!m_current_tsc)
+ m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
+else if (!m_current_tsc->InRange(m_pos))
+ m_current_tsc =
+ IsForwards() ? m_current_tsc->Next() : m_current_tsc->Prev();
+
if (!m_ignore_errors && IsError())
return true;
if (GetInstructionControlFlowType() & m_granularity)
@@ -61,20 +69,23 @@
switch (origin) {
case TraceCursor::SeekType::Set:
m_pos = fitPosToBounds(offset);
+m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
return m_pos;
case TraceCursor::SeekType::End:
m_pos = fitPosToBounds(offset + last_index);
+m_current_tsc = m_decoded_thread_sp->CalculateTscRange(last_index - m_pos);
return last_index - m_pos;
case TraceCursor::SeekType::Current:
int64_t new_pos = fitPosToBounds(offset + m_pos);
int64_t dist = m_pos - new_pos;
m_pos = new_pos;
+m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
return std::abs(dist);
}
}
bool TraceCursorIntelPT::IsError() {
- return m_decoded_thread_sp->GetInstructions()[m_pos].IsError();
+ return m_decoded_thread_sp->IsInstructionAnError(m_pos);
}
const char *TraceCursorIntelPT::GetError() {
@@ -85,10 +96,14 @@
return m_decoded_thread_sp->GetInstructions()[m_pos].GetLoadAddress();
}
-Optional TraceCursorIntelPT::GetCounter(lldb::TraceCounter counter_type) {
+Optional
+TraceCursorIntelPT::GetCounter(lldb::TraceCounter counter_type) {
switch (counter_type) {
-case lldb::eTraceCounterTSC:
- return m_decoded_thread_sp->GetInstructions()[m_pos].GetTimestampCounter();
+ case lldb::eTraceCounterTSC:
+if (m_current_tsc)
+ return m_current_tsc->GetTsc();
+else
+ return llvm::None;
}
}
Index: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
===
--- lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
+++ lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
@@ -62,9 +62,6 @@
/// As mentioned, any gap is represented as an error in this class.
class IntelPTInstruction {
public:
- IntelPTInstruction(const pt_insn &pt_insn, uint64_t timestamp)
- : m_pt_insn(pt_insn), m_timestamp(timestamp), m_is_error(false) {}
-
IntelPTInstruction(const pt_insn &pt_insn)
: m_pt_insn(pt_insn), m_is_error(false) {}
@@ -85,13 +82,6 @@
/// Get the size in bytes of an instance of this class
static size_t GetMemoryUsage();
- /// Get the timestamp associated with the current instruction. The timestamp
- /// is similar to what a rdtsc instruction would return
[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction
zrthxn added inline comments.
Comment at: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp:45-52
+if (!m_current_tsc)
+ m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
+else if (!m_current_tsc->InRange(m_pos)) {
+ if (m_pos > m_current_tsc->GetEnd())
+m_current_tsc = m_current_tsc->Next();
+ if (m_pos < m_current_tsc->GetStart())
+m_current_tsc = m_current_tsc->Prev();
wallace wrote:
> No need to do `m_current_tsc =
> m_decoded_thread_sp->CalculateTscRange(m_pos);` because its value has already
> been calculated in the constructor. We can simplify this as well
It is possible that when TraceCursorIntelPT is created the m_current_tsc is
None, for example when just started the trace and tried to dump instructions...
But then if a tsc is emitted later, this would cause it to remain None since we
don't re-calculate it if it was initially None
Comment at: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp:102-103
-Optional TraceCursorIntelPT::GetCounter(lldb::TraceCounter
counter_type) {
+Optional
+TraceCursorIntelPT::GetCounter(lldb::TraceCounter counter_type) {
+ if (!m_current_tsc)
wallace wrote:
> are you using git clang-format? I'm curious why this line changed
Yes I am. I think its because its longer than 80 chars.
Comment at: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp:104-110
+ if (!m_current_tsc)
+return None;
+
switch (counter_type) {
-case lldb::eTraceCounterTSC:
- return
m_decoded_thread_sp->GetInstructions()[m_pos].GetTimestampCounter();
+ case lldb::eTraceCounterTSC:
+return m_current_tsc->GetTsc();
}
wallace wrote:
>
m_current_tsc is already checked at the beginning of this function
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122603/new/
https://reviews.llvm.org/D122603
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction
zrthxn updated this revision to Diff 419347.
zrthxn marked 23 inline comments as done.
zrthxn added a comment.
Included requested changes, removed extra members
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122603/new/
https://reviews.llvm.org/D122603
Files:
lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
@@ -119,8 +119,9 @@
s.Printf(" Total number of instructions: %zu\n", insn_len);
s.Printf(" Total approximate memory usage: %0.2lf KiB\n",
(double)mem_used / 1024);
- s.Printf(" Average memory usage per instruction: %zu bytes\n",
- mem_used / insn_len);
+ if (insn_len != 0)
+s.Printf(" Average memory usage per instruction: %zu bytes\n",
+ mem_used / insn_len);
return;
}
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
@@ -42,6 +42,8 @@
DecodedThreadSP m_decoded_thread_sp;
/// Internal instruction index currently pointing at.
size_t m_pos;
+ /// Tsc range covering the current instruction.
+ llvm::Optional m_current_tsc;
};
} // namespace trace_intel_pt
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
@@ -23,6 +23,7 @@
assert(!m_decoded_thread_sp->GetInstructions().empty() &&
"a trace should have at least one instruction or error");
m_pos = m_decoded_thread_sp->GetInstructions().size() - 1;
+ m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
}
size_t TraceCursorIntelPT::GetInternalInstructionSize() {
@@ -40,6 +41,13 @@
while (canMoveOne()) {
m_pos += IsForwards() ? 1 : -1;
+
+if (!m_current_tsc)
+ m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
+else if (!m_current_tsc->InRange(m_pos))
+ m_current_tsc =
+ IsForwards() ? m_current_tsc->Next() : m_current_tsc->Prev();
+
if (!m_ignore_errors && IsError())
return true;
if (GetInstructionControlFlowType() & m_granularity)
@@ -61,20 +69,23 @@
switch (origin) {
case TraceCursor::SeekType::Set:
m_pos = fitPosToBounds(offset);
+m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
return m_pos;
case TraceCursor::SeekType::End:
m_pos = fitPosToBounds(offset + last_index);
+m_current_tsc = m_decoded_thread_sp->CalculateTscRange(last_index - m_pos);
return last_index - m_pos;
case TraceCursor::SeekType::Current:
int64_t new_pos = fitPosToBounds(offset + m_pos);
int64_t dist = m_pos - new_pos;
m_pos = new_pos;
+m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
return std::abs(dist);
}
}
bool TraceCursorIntelPT::IsError() {
- return m_decoded_thread_sp->GetInstructions()[m_pos].IsError();
+ return m_decoded_thread_sp->IsInstructionAnError(m_pos);
}
const char *TraceCursorIntelPT::GetError() {
@@ -85,10 +96,14 @@
return m_decoded_thread_sp->GetInstructions()[m_pos].GetLoadAddress();
}
-Optional TraceCursorIntelPT::GetCounter(lldb::TraceCounter counter_type) {
+Optional
+TraceCursorIntelPT::GetCounter(lldb::TraceCounter counter_type) {
+ if (!m_current_tsc)
+return None;
+
switch (counter_type) {
-case lldb::eTraceCounterTSC:
- return m_decoded_thread_sp->GetInstructions()[m_pos].GetTimestampCounter();
+ case lldb::eTraceCounterTSC:
+return m_current_tsc->GetTsc();
}
}
Index: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
===
--- lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
+++ lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
@@ -62,9 +62,6 @@
/// As mentioned, any gap is represented as an error in this class.
class IntelPTInstruction {
public:
- IntelPTInstruction(const pt_insn &pt_insn, uint64_t timestamp)
- : m_pt_insn(pt_insn), m_timestamp(timestamp), m_is_error(false) {}
-
IntelPTInstruction(const pt_insn &pt_insn)
: m_pt_insn(pt_insn), m_is_error(false) {}
@@ -85,13 +82,6 @@
/// Get the size in bytes of an instance of this class
static size_t GetMemoryUsage();
- /// Get the timestamp associated with the current instruction. The timestamp
- /// is
[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction
wallace requested changes to this revision.
wallace added a comment.
This revision now requires changes to proceed.
Some calculations are wrong, but overall this is good. We are very close!
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:112-113
+DecodedThread::CalculateTscRange(size_t insn_index) const {
+ if (m_instruction_timestamps.empty())
+return None;
+
now that I think of this, you can delete this, because if the map is empty,
this function will return in line 117
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:135-138
+: m_thread_sp(thread_sp), m_last_tsc(None) {}
DecodedThread::DecodedThread(ThreadSP thread_sp, Error &&error)
+: m_thread_sp(thread_sp), m_last_tsc(None) {
undo these two lines
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:159
+DecodedThread::TscRange::TscRange(std::map::const_iterator
it,
+ const DecodedThread &ref)
+: m_it(it), m_decoded_thread(ref) {
decoded_thread instead of ref
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:161-162
+: m_it(it), m_decoded_thread(ref) {
+ m_start_index = it->first;
+ m_tsc = it->second;
+
delete
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:164-168
+ auto end = m_decoded_thread.m_instruction_timestamps.end();
+ if (it != end)
+m_end_index = (++it--)->first - 1;
+ else
+m_end_index = end->first;
seeing ++ and -- is very hard to read. I also prefer not to modify the `it`
variable for cleanness. Also doing end->first might crash the program. I'm
writing here a correct version
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:173
+
+size_t DecodedThread::TscRange::GetStart() const { return m_start_index; }
+
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:178
+bool DecodedThread::TscRange::InRange(size_t insn_index) {
+ if (insn_index < m_end_index && insn_index > m_start_index)
+return true;
The comparison is not right. let's use <= in a specific order to make it easier
to read
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:186
+return None;
+ return TscRange(++m_it, m_decoded_thread);
+}
As m_it is valid, doing the comparison `m_it ==
m_decoded_thread.m_instruction_timestamps.end()` will always return false.
Remember that .end() will return a fake iterator that points to no value.
Besides that, don't modify m_it. Let's better create a new iterator
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp:190-192
+ if (m_it == m_decoded_thread.m_instruction_timestamps.end())
+return None;
+ return TscRange(--m_it, m_decoded_thread);
Similarly, this has to be improved. I also like to put `--it` statements in
their own line to make it easier to read.
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:142
+ /// \class TscRange
+ /// Class that represents the instruction range associated with a given TSC.
+ /// It provides efficient iteration to the previous or next TSC range in the
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:147-148
+ /// TSC timestamps are emitted by the decoder infrequently, which means
+ /// that each TSC covers a range of instruction indices, which we can use to
+ /// speed up TSC lookups.
+ class TscRange {
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:149
+ /// speed up TSC lookups.
+ class TscRange {
+ public:
Move this class to the beginning of the public section of DecodedThread for
easier discoverability
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:151
+ public:
+// Check if current TSC range covers given instruction index.
+bool InRange(size_t insn_index);
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:167-173
+TscRange &operator=(const TscRange &r) {
+ m_it = r.m_it;
+ m_tsc = r.m_tsc;
+ m_start_index = r.m_start_index;
+ m_end_index = r.m_end_index;
+ return *this;
+}
let's better delete this. It adds some maintenance cost with little benefits
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:178
+
+// Construct TscRange respecting bounds of timestamp map in thread
+TscRange(std::map::const_iterator it,
This comment is hard to follow. Let's just delete it because it's a private
constructor
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:185-188
+/// The TSC v
[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction
zrthxn updated this revision to Diff 418981.
zrthxn added a comment.
Prevent crash on printing info when we have 0 instructions
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122603/new/
https://reviews.llvm.org/D122603
Files:
lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
@@ -119,8 +119,9 @@
s.Printf(" Total number of instructions: %zu\n", insn_len);
s.Printf(" Total approximate memory usage: %0.2lf KiB\n",
(double)mem_used / 1024);
- s.Printf(" Average memory usage per instruction: %zu bytes\n",
- mem_used / insn_len);
+ if (insn_len != 0)
+s.Printf(" Average memory usage per instruction: %zu bytes\n",
+(mem_used - *raw_size) / insn_len);
return;
}
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
@@ -42,6 +42,8 @@
DecodedThreadSP m_decoded_thread_sp;
/// Internal instruction index currently pointing at.
size_t m_pos;
+ /// Current instruction timestamp.
+ llvm::Optional m_current_tsc;
};
} // namespace trace_intel_pt
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
@@ -23,6 +23,7 @@
assert(!m_decoded_thread_sp->GetInstructions().empty() &&
"a trace should have at least one instruction or error");
m_pos = m_decoded_thread_sp->GetInstructions().size() - 1;
+ m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
}
size_t TraceCursorIntelPT::GetInternalInstructionSize() {
@@ -40,6 +41,16 @@
while (canMoveOne()) {
m_pos += IsForwards() ? 1 : -1;
+
+if (!m_current_tsc)
+ m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
+else if (!m_current_tsc->InRange(m_pos)) {
+ if (m_pos > m_current_tsc->GetEnd())
+m_current_tsc = m_current_tsc->Next();
+ if (m_pos < m_current_tsc->GetStart())
+m_current_tsc = m_current_tsc->Prev();
+}
+
if (!m_ignore_errors && IsError())
return true;
if (GetInstructionControlFlowType() & m_granularity)
@@ -61,20 +72,23 @@
switch (origin) {
case TraceCursor::SeekType::Set:
m_pos = fitPosToBounds(offset);
+m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
return m_pos;
case TraceCursor::SeekType::End:
m_pos = fitPosToBounds(offset + last_index);
+m_current_tsc = m_decoded_thread_sp->CalculateTscRange(last_index - m_pos);
return last_index - m_pos;
case TraceCursor::SeekType::Current:
int64_t new_pos = fitPosToBounds(offset + m_pos);
int64_t dist = m_pos - new_pos;
m_pos = new_pos;
+m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
return std::abs(dist);
}
}
bool TraceCursorIntelPT::IsError() {
- return m_decoded_thread_sp->GetInstructions()[m_pos].IsError();
+ return m_decoded_thread_sp->IsInstructionAnError(m_pos);
}
const char *TraceCursorIntelPT::GetError() {
@@ -85,10 +99,14 @@
return m_decoded_thread_sp->GetInstructions()[m_pos].GetLoadAddress();
}
-Optional TraceCursorIntelPT::GetCounter(lldb::TraceCounter counter_type) {
+Optional
+TraceCursorIntelPT::GetCounter(lldb::TraceCounter counter_type) {
+ if (!m_current_tsc)
+return None;
+
switch (counter_type) {
-case lldb::eTraceCounterTSC:
- return m_decoded_thread_sp->GetInstructions()[m_pos].GetTimestampCounter();
+ case lldb::eTraceCounterTSC:
+return m_current_tsc->GetTsc();
}
}
Index: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
===
--- lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
+++ lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
@@ -62,9 +62,6 @@
/// As mentioned, any gap is represented as an error in this class.
class IntelPTInstruction {
public:
- IntelPTInstruction(const pt_insn &pt_insn, uint64_t timestamp)
- : m_pt_insn(pt_insn), m_timestamp(timestamp), m_is_error(false) {}
-
IntelPTInstruction(const pt_insn &pt_insn)
: m_pt_insn(pt_insn), m_is_error(false) {}
@@ -85,13 +82,6 @@
/// Get the size in bytes of an instance of this class
static size_t GetMemoryUsage();
- /// Get the timestam
[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction
zrthxn updated this revision to Diff 418977.
zrthxn added a comment.
Update memory calc function
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122603/new/
https://reviews.llvm.org/D122603
Files:
lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
Index: lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceIntelPT.cpp
@@ -120,7 +120,7 @@
s.Printf(" Total approximate memory usage: %0.2lf KiB\n",
(double)mem_used / 1024);
s.Printf(" Average memory usage per instruction: %zu bytes\n",
- mem_used / insn_len);
+ (mem_used - *raw_size) / insn_len);
return;
}
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
@@ -42,6 +42,8 @@
DecodedThreadSP m_decoded_thread_sp;
/// Internal instruction index currently pointing at.
size_t m_pos;
+ /// Current instruction timestamp.
+ llvm::Optional m_current_tsc;
};
} // namespace trace_intel_pt
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
@@ -23,6 +23,7 @@
assert(!m_decoded_thread_sp->GetInstructions().empty() &&
"a trace should have at least one instruction or error");
m_pos = m_decoded_thread_sp->GetInstructions().size() - 1;
+ m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
}
size_t TraceCursorIntelPT::GetInternalInstructionSize() {
@@ -40,6 +41,16 @@
while (canMoveOne()) {
m_pos += IsForwards() ? 1 : -1;
+
+if (!m_current_tsc)
+ m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
+else if (!m_current_tsc->InRange(m_pos)) {
+ if (m_pos > m_current_tsc->GetEnd())
+m_current_tsc = m_current_tsc->Next();
+ if (m_pos < m_current_tsc->GetStart())
+m_current_tsc = m_current_tsc->Prev();
+}
+
if (!m_ignore_errors && IsError())
return true;
if (GetInstructionControlFlowType() & m_granularity)
@@ -61,20 +72,23 @@
switch (origin) {
case TraceCursor::SeekType::Set:
m_pos = fitPosToBounds(offset);
+m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
return m_pos;
case TraceCursor::SeekType::End:
m_pos = fitPosToBounds(offset + last_index);
+m_current_tsc = m_decoded_thread_sp->CalculateTscRange(last_index - m_pos);
return last_index - m_pos;
case TraceCursor::SeekType::Current:
int64_t new_pos = fitPosToBounds(offset + m_pos);
int64_t dist = m_pos - new_pos;
m_pos = new_pos;
+m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
return std::abs(dist);
}
}
bool TraceCursorIntelPT::IsError() {
- return m_decoded_thread_sp->GetInstructions()[m_pos].IsError();
+ return m_decoded_thread_sp->IsInstructionAnError(m_pos);
}
const char *TraceCursorIntelPT::GetError() {
@@ -85,10 +99,14 @@
return m_decoded_thread_sp->GetInstructions()[m_pos].GetLoadAddress();
}
-Optional TraceCursorIntelPT::GetCounter(lldb::TraceCounter counter_type) {
+Optional
+TraceCursorIntelPT::GetCounter(lldb::TraceCounter counter_type) {
+ if (!m_current_tsc)
+return None;
+
switch (counter_type) {
-case lldb::eTraceCounterTSC:
- return m_decoded_thread_sp->GetInstructions()[m_pos].GetTimestampCounter();
+ case lldb::eTraceCounterTSC:
+return m_current_tsc->GetTsc();
}
}
Index: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
===
--- lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
+++ lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
@@ -62,9 +62,6 @@
/// As mentioned, any gap is represented as an error in this class.
class IntelPTInstruction {
public:
- IntelPTInstruction(const pt_insn &pt_insn, uint64_t timestamp)
- : m_pt_insn(pt_insn), m_timestamp(timestamp), m_is_error(false) {}
-
IntelPTInstruction(const pt_insn &pt_insn)
: m_pt_insn(pt_insn), m_is_error(false) {}
@@ -85,13 +82,6 @@
/// Get the size in bytes of an instance of this class
static size_t GetMemoryUsage();
- /// Get the timestamp associated with the current instruction. The timestamp
- /// is similar to what a rdtsc instruction would return.
- ///
- /// \return
- /// The timestamp or \b llvm::None if not
[Lldb-commits] [PATCH] D122603: [intelpt] Refactor timestamps out of IntelPTInstruction
zrthxn updated this revision to Diff 418975.
zrthxn marked 7 inline comments as done.
zrthxn retitled this revision from "[wip][intelpt] Refactor timestamps out of
`IntelPTInstruction`" to "[intelpt] Refactor timestamps out of
IntelPTInstruction".
zrthxn added a comment.
Change TscRange to class
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122603/new/
https://reviews.llvm.org/D122603
Files:
lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.h
@@ -42,6 +42,8 @@
DecodedThreadSP m_decoded_thread_sp;
/// Internal instruction index currently pointing at.
size_t m_pos;
+ /// Current instruction timestamp.
+ llvm::Optional m_current_tsc;
};
} // namespace trace_intel_pt
Index: lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
===
--- lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
+++ lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
@@ -23,6 +23,7 @@
assert(!m_decoded_thread_sp->GetInstructions().empty() &&
"a trace should have at least one instruction or error");
m_pos = m_decoded_thread_sp->GetInstructions().size() - 1;
+ m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
}
size_t TraceCursorIntelPT::GetInternalInstructionSize() {
@@ -40,6 +41,16 @@
while (canMoveOne()) {
m_pos += IsForwards() ? 1 : -1;
+
+if (!m_current_tsc)
+ m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
+else if (!m_current_tsc->InRange(m_pos)) {
+ if (m_pos > m_current_tsc->GetEnd())
+m_current_tsc = m_current_tsc->Next();
+ if (m_pos < m_current_tsc->GetStart())
+m_current_tsc = m_current_tsc->Prev();
+}
+
if (!m_ignore_errors && IsError())
return true;
if (GetInstructionControlFlowType() & m_granularity)
@@ -61,20 +72,23 @@
switch (origin) {
case TraceCursor::SeekType::Set:
m_pos = fitPosToBounds(offset);
+m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
return m_pos;
case TraceCursor::SeekType::End:
m_pos = fitPosToBounds(offset + last_index);
+m_current_tsc = m_decoded_thread_sp->CalculateTscRange(last_index - m_pos);
return last_index - m_pos;
case TraceCursor::SeekType::Current:
int64_t new_pos = fitPosToBounds(offset + m_pos);
int64_t dist = m_pos - new_pos;
m_pos = new_pos;
+m_current_tsc = m_decoded_thread_sp->CalculateTscRange(m_pos);
return std::abs(dist);
}
}
bool TraceCursorIntelPT::IsError() {
- return m_decoded_thread_sp->GetInstructions()[m_pos].IsError();
+ return m_decoded_thread_sp->IsInstructionAnError(m_pos);
}
const char *TraceCursorIntelPT::GetError() {
@@ -85,10 +99,14 @@
return m_decoded_thread_sp->GetInstructions()[m_pos].GetLoadAddress();
}
-Optional TraceCursorIntelPT::GetCounter(lldb::TraceCounter counter_type) {
+Optional
+TraceCursorIntelPT::GetCounter(lldb::TraceCounter counter_type) {
+ if (!m_current_tsc)
+return None;
+
switch (counter_type) {
-case lldb::eTraceCounterTSC:
- return m_decoded_thread_sp->GetInstructions()[m_pos].GetTimestampCounter();
+ case lldb::eTraceCounterTSC:
+return m_current_tsc->GetTsc();
}
}
Index: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
===
--- lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
+++ lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
@@ -62,9 +62,6 @@
/// As mentioned, any gap is represented as an error in this class.
class IntelPTInstruction {
public:
- IntelPTInstruction(const pt_insn &pt_insn, uint64_t timestamp)
- : m_pt_insn(pt_insn), m_timestamp(timestamp), m_is_error(false) {}
-
IntelPTInstruction(const pt_insn &pt_insn)
: m_pt_insn(pt_insn), m_is_error(false) {}
@@ -85,13 +82,6 @@
/// Get the size in bytes of an instance of this class
static size_t GetMemoryUsage();
- /// Get the timestamp associated with the current instruction. The timestamp
- /// is similar to what a rdtsc instruction would return.
- ///
- /// \return
- /// The timestamp or \b llvm::None if not available.
- llvm::Optional GetTimestampCounter() const;
-
/// Get the \a lldb::TraceInstructionControlFlowType categories of the
/// instruction.
///
@@ -113,7 +103,6 @@
// When adding new members to this class, make sure to update
// IntelPTInstruction::GetNonErrorMemoryUsage() if needed.
pt_insn m_pt_insn;
- llvm::Optional m_timestamp;
bool m_is_e
