[Lldb-commits] [PATCH] D103588: [trace] Create a top-level instruction class

[Vedant Kumar via Phabricator via lldb-commits]

vsk added a comment.

In D103588#2806512 <https://reviews.llvm.org/D103588#2806512>, @wallace wrote:

> Right now we haven't implemented lazy decoding; we are decoding the entire trace. But that's just because so far we have been working with small traces. As we progress in this work, we'll start working with larger traces and we'll have to do implement the lazy decoding, for which the TraverseInstructions API will come handy.

This approach - of prototyping trace analyses on a vector<TraceInstruction> representation and then rewriting them later when scaling issues arise - doesn't sound good to me. Even for something simple, like finding a backtrace, the window of instructions needed for analysis can easily grow to a point where the full vector<TraceInstruction> can't be maintained in memory.

To clarify: I think that's a perfectly reasonable way to prototype trace analyses, but I don't think that kind of prototyping should be done at the SB API level, as there are extra bincompat & portability concerns here.

>> What alternatives to the vector<TraceInstruction> representation have you considered? One idea might be to implement your program analyses on top of a generic interface for navigating forward/backward through a trace and extracting info about the instruction via a set of API calls; this leaves the actual in-memory representation of "TraceInstruction" unspecified.
>
> That's exactly what I described as the TraverseInstructions method :) The intel-pt plugin or any other trace plugin could have an internal representation for traces, but we still need a generic TraceInstruction object for consumers that want to be agnostic of the trace technology.

It's not at all clear to me that lldb needs to surface a generic TraceInstruction object, with metadata like m_byte_size, m_inst_type, etc. stored inline. It seems more flexible to instead vend a cursor (just an integer) that indicates a position in the trace; a separate set of APIs could answer queries given a cursor, e.g. 'getInstTypeAtCursor(u64 cursor)'.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D103588/new/

https://reviews.llvm.org/D103588