wallace added inline comments.
================
Comment at: lldb/include/lldb/Target/TraceHTR.h:29
+/// Metadata is initially populated in Layer 1 and merged as blocks are merged
+class HTRBlockMetadata {
+public:
----------------
This is a very expensive structure if you are going to have for Layer 1, which
is just a bunch of instructions => m_num_instructions will always be 1 and
m_func_calls will have 0 or 1 entries.
I'd suggest creating two classes:
InstructionLayer (layer 0), where you could assume that m_num_instructions is 1
and instead of having the map m_func_calls, you can have an
Optional<ConstString> for the function call, or you could also have a map
(address -> ConstString) at the layer level that could use to look up function
calls instead of storing this information in each instruction. If you follow
the latter advice, the memory usage of Layer 0 would have small overhead, i.e.
InstructionLayer = Instruction list + function map
BlockLayer (layer 1+), here you have blocks of 1 or more instructions. Assuming
that the number of blocks will be small, you could store more metadata in each
block and here statistics like number of function calls make more sense.
================
Comment at: lldb/include/lldb/Target/TraceHTR.h:41
+ size_t m_num_instructions;
+ std::unordered_map<std::string, size_t> m_func_calls;
+ friend llvm::json::Value toJSON(const HTRBlockMetadata &metadata);
----------------
clayborg wrote:
> Use ConstString here instead of std::string. std::string will make a copy of
> the string. ConstString objects should always be used for storing function
> and type names as a constant string pool will unique any strings that are
> placed into a ConstString. This will avoid consuming more memory for these
> traces and ConstString also is much more efficient for comparing strings for
> equality if that is needed anywhere.
See my comment above, this is too memory expensive
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105741/new/
https://reviews.llvm.org/D105741
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
