[Lldb-commits] [PATCH] D40214: performance: Prevent needless DWARFCompileUnit::Clear() on freshly ctor-ed object

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added a comment.

Thanks for the review but then it would become a performance regression, not the performance improvement I was trying to make.
Withdrawing this patch.



================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp:74
                                lldb::offset_t *offset_ptr) {
-  Clear();
-
+  assert(m_offset == DW_INVALID_OFFSET);
   m_offset = *offset_ptr;
----------------
clayborg wrote:
> Make this static as mentioned in above comment. Assertions are ok to detect things in debug build, but please use lldbassert and make sure it returns an empty shared pointer if things fail (code must function properly when assert is not compiled in the program.
In such case each Extract() call will always make a new allocation of DWARFCompileUnit instance which is much more performance expensive that the double in-place reinitialization this patch was trying to avoid.



================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp:294-297
       if (!cu.unique())
         cu.reset(new DWARFCompileUnit(dwarf2Data));
+      else
+        cu->Clear();
----------------
clayborg wrote:
> I don't see this code in top of tree? My DWARFDebugInfo.cpp ends at line 259
I see it in:
[[ https://github.com/llvm-mirror/lldb/blob/master/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp#L295 ]]
and also in:
[[ https://llvm.org/svn/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp ]]



https://reviews.llvm.org/D40214