This is an archive of the discontinued LLVM Phabricator instance.

[trace][intel pt] Add TSC timestamps
ClosedPublic

Authored by wallace on Jul 19 2021, 5:37 PM.

Details

Summary

I've just added the GetTimestampCounter method to Trace.h, which returns an Optional of a timestamp counter. Major CPU architectures support timestamp counters, which fortunately are invariant in modern chips (i.e. they are synchronized between logical CPUs and increase at a constant rate). This helps with having accurate timestamps even when the the thread is jumping around CPUs.

A followup diff would be to add a GetRealTime method, which would ask the trace plugin to convert a TSC to any of ms/ns/us or just to return the real time if available in the actual trace.

Tools could be built upon any of those two APIs, being GetTimestampCounter way more accurate in terms of keeping relative times.

I also implemented the specific bits for Intel PT. Now the "trace start" command accepts two params: --tsc which enables the generation of timestamp counters, and --psb_period, which is the simplest way to tweak how often these timestamps are generated. There are other ways to increase the resolution of timestamps: CYC and MTC packets, but that can be left for a follow up diff to keep this one short.

wallace@devbig418 ~/llvm-sand/build/Release/fbcode-x86_64/toolchain ▶ ./bin/lldb ~/a.out 
(lldb) target create "/home/wallace/a.out"
Current executable set to '/home/wallace/a.out' (x86_64).
(lldb) b main
Breakpoint 1: where = a.out`main + 9 at main.cpp:6:15, address = 0x0000000000400c5f
(lldb) r
Process 1130363 launched: '/home/wallace/a.out' (x86_64)
Process 1130363 stopped
* thread #1, name = 'a.out', stop reason = breakpoint 1.1
    frame #0: 0x0000000000400c5f a.out`main at main.cpp:6:15
   3    using namespace std;
   4   
   5    int main() {
-> 6      vector<int> s;
   7      s.push_back(1);
   8      s.push_back(2);
   9      cout << s.size() << endl;
(lldb) thread trace start --tsc
(lldb) n
Process 1130363 stopped
* thread #1, name = 'a.out', stop reason = step over
    frame #0: 0x0000000000400c6b a.out`main at main.cpp:7:15
   4   
   5    int main() {
   6      vector<int> s;
-> 7      s.push_back(1);
   8      s.push_back(2);
   9      cout << s.size() << endl;
   10     int x = 1;
(lldb) thread trace dump instructions --tsc -f
thread #1: tid = 1130363
  a.out`main + 9 at main.cpp:6:15
    [ 0] [tsc=0x005a8eadd08764ba] 0x0000000000400c5f    leaq   -0x30(%rbp), %rax
    [ 1] [tsc=0x005a8eadd09356a8] 0x0000000000400c63    movq   %rax, %rdi
    [ 2] [tsc=0x005a8eadd09356a8] 0x0000000000400c66    callq  0x400d9c                  ; std::vector<int, std::allocator<int> >::vector at stl_vector.h:391:7
  ...

Diff Detail

Event Timeline

wallace created this revision.Jul 19 2021, 5:37 PM
wallace retitled this revision from [intel pt] Add TSC timestamps to [trace][intel pt] Add TSC timestamps.Jul 19 2021, 7:15 PM
wallace edited the summary of this revision. (Show Details)
wallace added a reviewer: clayborg.
wallace published this revision for review.Jul 19 2021, 7:15 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 19 2021, 7:15 PM
clayborg requested changes to this revision.Jul 19 2021, 7:38 PM

Man cases of having variables named "tsc" or "m_tsc" when it might be clear to have them named "enable_tsc" and "m_enable_tsc".

lldb/source/Commands/Options.td
1071

You mentioned "--psb_period" in your description, but that isn't here. Just wanted to make sure that is on purpose. And if you do add it it should be --psb-period with - and not a _

lldb/source/Plugins/Process/Linux/IntelPTManager.cpp
58

Don't use auto here? More readable

87

Is "buffer" binary or text? If text, then this is fine.

104

Do you want an else clause here to error check "ZeroOne" in case you get something like 12 back?

lldb/source/Plugins/Process/Linux/IntelPTManager.h
85

rename "tsc" to "enable_tsc"?

lldb/source/Plugins/Trace/intel-pt/TraceIntelPTConstants.h
21

"kDefaultTSCValue"? The name above doesn't really convey what this variable is

22

kDefaultPSBPeriod?

This revision now requires changes to proceed.Jul 19 2021, 7:38 PM
wallace updated this revision to Diff 360265.Jul 20 2021, 2:00 PM

address comments + add tests

wallace added inline comments.Jul 20 2021, 3:30 PM
lldb/source/Commands/Options.td
1071

that's in the TraceIntelPTOptions.td file :), as it's intel pt specific

clayborg accepted this revision.Jul 20 2021, 4:01 PM

Looks good

This revision is now accepted and ready to land.Jul 20 2021, 4:01 PM
This revision was landed with ongoing or failed builds.Jul 20 2021, 4:37 PM
This revision was automatically updated to reflect the committed changes.