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

[walter erquinigo via Phabricator via lldb-commits]

wallace added a comment.

Btw, thanks for the conversation. This is being really helpful.

> I'm not convinced that retaining only 1 decoded instruction in memory at a time (under a TraverseInstructions callback) will be sufficient for the use cases you've outlined. Let's say the user sets a few breakpoints, then repeatedly does "rev-continue" followed by "bt". If lldb only holds a max of 1 decoded instruction in memory, it would need to repeatedly decode parts of the trace to evaluate those user commands, which would be slow as you pointed out earlier. OTOH there's not enough memory to retain all of the decoded instructions.
>
> It seems like we need a way for generic trace analyses to request _batches_ of decoded trace data at a time, lazily demanding more from the trace plugin only if the analysis requires it to proceed, while freeing any stale decoded data it's not using.
>
> Moreover, there doesn't seem to be a hard requirement that each trace plugin specialize a TraceInstruction class. For example, to support that breakpoint/rev-continue/bt workflow, you really only need a list of the places where _control flow changed_. So the generic layer could request that data from a trace plugin ('plugin->decodeControlFlowChanges(startPos, stopPos)'), then query more specific APIs that answer questions about how many control flow changes there were, and what the inst type + load address was at each control flow change point. Each plugin implementation can pick the most efficient way to implement those narrower APIs, instead of trying to cram all potentially useful information into a single TraceInst class.
>
> The reason why I believe this to be such a crucial part of the design is because of the scaling problem I mentioned earlier. A single 1.5GHz core running at IPC=2 yields 3*10^9 instructions/second, so we hit "gigantic" all too quickly!

Yes, I agree with everything you said. It will really make the implementation simpler and more generic if we have these accessors based on indices instead of a struct/class, and we let each plug-in manage its data and its cache their own way. 
`decodeControlFlowChanges` is definitely a good idea, and other similar interfaces could be created following the same approach.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103588