[Lldb-commits] [PATCH] D122293: [wip][intelpt] Refactoring instruction decoding for flexibility

[Alisamar Husain via Phabricator via lldb-commits]

zrthxn added inline comments.


================
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:172
   std::vector<IntelPTInstruction> m_instructions;
+  std::vector<llvm::Error> m_errors;
+
----------------
jj10306 wrote:
> wallace wrote:
> > you need to have something like
> >   std::unordered_map<uint64_t, llvm::Error> m_errors;
> > 
> > that way, you'll be able to quickly look for the error associated with an instruction index. The IntelPTInstruction int his case, instead of storing the Error, can just store one bit of information has_error = true/false;
> nit: from https://llvm.org/docs/CodingStandards.html#c-standard-library 
> prefer `llvm::DenseMap` over `std::map`'s unless there's a specific reason not to.
> 
> Don't forget to update `CalculateApproximateMemoryUsage()` as well! Also, besides for being inline with the coding standards I linked above, using `llvm::DenseMap` here has the actual advantage that it exposes its approximate size via  `getMemorySize()`, whereas there is no easy way to get the size of `std::map`.
DenseMap looks interesting we should try that.

Yes i will update the mem calculation and make it more accurate so some refactor will be needed. That'll be the next small patch once this works. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122293