This is an archive of the discontinued LLVM Phabricator instance.

[lldb][intelpt] Remove `IntelPTInstruction` and move methods to `DecodedThread`
ClosedPublic

Authored by zrthxn on Apr 3 2022, 2:53 AM.

Details

Summary

This is to reduce the size of the trace further and has appreciable results.

Diff Detail

Event Timeline

zrthxn created this revision.Apr 3 2022, 2:53 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 3 2022, 2:53 AM
zrthxn requested review of this revision.Apr 3 2022, 2:53 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 3 2022, 2:53 AM
zrthxn updated this revision to Diff 420245.Apr 4 2022, 11:01 AM

Change in memory use is appreciable.

thread #1: tid = 40371
  Raw trace size: 4 KiB
  Total number of instructions: 145011
  Total approximate memory usage: 1840.96 KiB
  Average memory usage per instruction: 13.00 bytes

  Number of TSC decoding errors: 0

Tests still need to be updated.

zrthxn updated this revision to Diff 420256.Apr 4 2022, 11:24 AM

Updated tests

wallace requested changes to this revision.Apr 4 2022, 9:00 PM

only mostly cosmetic changes needed. Thanks for this. I'm glad that we are bringing the usage down

lldb/source/Plugins/Trace/intel-pt/DecodedThread.cpp
46–47
54–58

let's give better names to these variables

62

we can stop using next_load_address because we now have access to the entire trace

176

you also need to include m_instruction_timestamps, probably m_instruction_timestamps.size() * (sizeof(size_t) + sizeof(uint64_t))

lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
127

lowercase tsc because it's a variable

129–136

I regret having created this method because it forces us to have a certain interface for the storage of the instructions. Let's just delete this method and create one new method

size_t GetInstructionsCount() const;

which should be able to replace all the external calls to this method

139
143–154

Now that DecodedThread knows everything, we can simplify this method a bit. First, we can remove the parameter next_load_address, because it can easily be gotten by checking the instruction at index insn_index + 1

193–199

+1

218
220
lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
23–30
This revision now requires changes to proceed.Apr 4 2022, 9:00 PM
zrthxn updated this revision to Diff 420401.Apr 5 2022, 1:06 AM
zrthxn marked 12 inline comments as done.

Address all comments

wallace requested changes to this revision.Apr 5 2022, 9:16 AM

just remove a small comment and good to go!

lldb/source/Plugins/Trace/intel-pt/DecodedThread.h
143–146

remove this

This revision now requires changes to proceed.Apr 5 2022, 9:16 AM
zrthxn updated this revision to Diff 420543.Apr 5 2022, 9:18 AM
zrthxn marked an inline comment as done.

Removed unnecessary comment

wallace accepted this revision.Apr 5 2022, 9:25 AM

great job!

This revision is now accepted and ready to land.Apr 5 2022, 9:25 AM