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



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
  • [Lldb-commits] [PATCH] D40... Greg Clayton via Phabricator via lldb-commits
    • [Lldb-commits] [PATCH... Jan Kratochvil via Phabricator via lldb-commits

Reply via email to