This is an archive of the discontinued LLVM Phabricator instance.

[trace][intel pt] Create a CPU change event and expose it in the dumper
ClosedPublic

Authored by wallace on Jul 7 2022, 6:45 PM.

Details

Summary

Thanks to fredzhou@fb.com for coming up with this feature.

When tracing in per-cpu mode, we have information of in which cpu we are execution each instruction, which comes from the context switch trace. This diff makes this information available as a cpu changed event, which an additional accessor in the cursor GetCPU(). As cpu changes are very infrequent, any consumer should listen to cpu change events instead of querying the actual cpu of a trace item. Once a cpu change event is seen, the consumer can invoke GetCPU() to get that information. Also, it's possible to invoke GetCPU() on an arbitrary instruction item, which will return the last cpu seen. However, this call is O(logn) and should be used sparingly.

Manually tested with a sample program that starts on cpu 52, then goes to 18, and then goes back to 52.

Diff Detail

Event Timeline

wallace created this revision.Jul 7 2022, 6:45 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 7 2022, 6:45 PM
wallace requested review of this revision.Jul 7 2022, 6:45 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 7 2022, 6:45 PM
wallace edited the summary of this revision. (Show Details)Jul 7 2022, 6:45 PM
jj10306 requested changes to this revision.Jul 13 2022, 7:35 AM

looks good overall, just a couple minor suggestions and questions

lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
288

Does this mean our trace will always start with the CPU change event or would it be possible that we won't know the CPU id until the first context switch occurs?

lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
81–88

afaiu, we use pretty much the same data structures to keep track of TSCs and CPU Ids. Is there a reason that the TSC structures live on the trace cursor whereas the CPU id structures live on the decoded thread itself?
Wondering if it would make more sense to move the TSC structures off of the trace cursor and be consistent with how it's done for CPU Ids.
wdyt?

lldb/source/Target/TraceDumper.cpp
137–141

What about making an EventToString that abstracts away the details of how to print an event? Basically the same as EventKindToString but have it take in an item instead of event kind.
Otherwise, we will have to continue adding logic here for each new event/any modifications to existing events

204–206

similar suggestions as above about potentially moving all of this logic into a single function, maybe in this case it could be EventToJson or something similar and it takes in a ref to the json as well as the item

This revision now requires changes to proceed.Jul 13 2022, 7:35 AM
wallace requested review of this revision.Jul 13 2022, 11:53 AM
wallace added inline comments.
lldb/source/Plugins/Trace/intel-pt/LibiptDecoder.cpp
288

for per-cpu tracing, we'll always have the cpu change event as the first trace item. For per-thread tracing, we won't have this

lldb/source/Plugins/Trace/intel-pt/TraceCursorIntelPT.cpp
81–88

I've been trying different ways of storing the data and exposing them, so that's why you see some inconsistencies. And yes, we should things the same way we we've doing it for CPUs, but I'll leave it for a future diff, in which we also decide how we are going to expose TSCs and nanoseconds in the trace cursor.

lldb/source/Target/TraceDumper.cpp
137–141

i think that won't be needed now because i don't see any other part of LLDB dumping this information in a string format. Other external consumers would probably have their own format for this data

204–206

again, i don't see this being reused in other parts of LLDB, so I don't think it's worth doing the abstractions

jj10306 accepted this revision.Jul 13 2022, 12:21 PM

thanks for following up on those questions, lgtm

This revision is now accepted and ready to land.Jul 13 2022, 12:21 PM
This revision was landed with ongoing or failed builds.Jul 13 2022, 12:26 PM
This revision was automatically updated to reflect the committed changes.