[Lldb-commits] [PATCH] optimize address range coalescing

[Stephen Wilson]

DWARFDebugAranges::Sort() calls std::stable_sort() over a set of address ranges
and then proceeds to collapse neighboring ranges together.

One problem with the current implementation is that it does an incomplete job.
When a pair of ranges are merged the next pair considered does not include the
just-merged range.  IOW, three consecutive ranges are never collapsed into one.

Another problem is that for each range merged we are calling
std::vector::erase() which "shifts" all remaining elements of the vector by one
position on every merge.  The end result (in the worst case) is a quadratic
algorithm -- not good when the input vector is large.

The following patch merges all consecutive ranges and removes the quadratic
behavior.  The implementation uses an auxiliary vector of indices in order to
remember all ranges that can be dropped, then performs the coalescing of ranges
in a single pass.

Instrumenting DWARFDebugAranges::Sort() with a timer and loading LLDB in LLDB
prior to this patch gives (on my prone-to-overheat 2GHz laptop):

   Current executable set to './lldb' (x86_64).
   (lldb) break set -n main
   Breakpoint created: 1: name = 'main', locations = 1
   (lldb) log timers enable
   (lldb) r
   Process 12840 launched: '/home/steve/development/build/llvm-git/Debug+Asserts/bin/lldb' (x86_64)
   (lldb) Process 12840 stopped
   * thread #1: tid = 0x3228, 0x0000000000411fed lldb`main + 32 at Driver.cpp:1275, stop reason = breakpoint 1.1
      1272	int
      1273	main (int argc, char const *argv[], const char *envp[])
      1274	{
   -> 1275	    SBDebugger::Initialize();
      1276    
      1277	    SBHostOS::ThreadCreated ("<lldb.driver.main-thread>");
      1278
   
   (lldb) log timers dump
   17.027432000 sec for void DWARFDebugAranges::Sort()
   ...

and after this patch:

   ...
   (lldb) log timers dump
   ...
   0.154519000 sec for void DWARFDebugAranges::Sort()
   ...



diff --git a/source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.cpp b/source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.cpp
index 8e3461d..2d69926 100644
--- a/source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.cpp
+++ b/source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.cpp
@@ -15,6 +15,7 @@
 #include <algorithm>
 
 #include "lldb/Core/Stream.h"
+#include "lldb/Core/Timer.h"
 
 #include "SymbolFileDWARF.h"
 #include "DWARFDebugInfo.h"
@@ -290,36 +291,46 @@ DWARFDebugAranges::AppendRange (dw_offset_t offset, dw_addr_t low_pc, dw_addr_t
 
 void
 DWARFDebugAranges::Sort()
-{    
-    // Sort our address range entries
+{
+    std::vector<size_t> indices;
+    size_t end;
+    Timer scoped_timer(__PRETTY_FUNCTION__, "%s this = %p",
+                       __PRETTY_FUNCTION__, this);
+
+    // Sort our address range entries.
     std::stable_sort (m_aranges.begin(), m_aranges.end(), RangeLessThan);
 
-    // Merge any entries that have the same offset and same start/end address
-    RangeColl::iterator pos = m_aranges.begin();
-    RangeColl::iterator end = m_aranges.end();
-    while (pos != end)
+    // Merge all neighbouring ranges into a single range and remember the
+    // indices of all ranges merged.
+    end = m_aranges.size();
+    for (size_t merge = 0, cursor = 1; cursor < end; ++cursor)
     {
-        RangeColl::iterator next_pos = pos + 1;
-        if (next_pos != end && 
-            pos->offset == next_pos->offset && 
-            pos->hi_pc == next_pos->lo_pc)
+        Range &r1 = m_aranges[merge];
+        Range &r2 = m_aranges[cursor];
+
+        if (r1.hi_pc == r2.lo_pc && r1.offset == r2.offset)
         {
-            // We have found an entry whose end address it he same as the
-            // next entry's start address and the offsets are the same so 
-            // we can merge these two entries.
-            pos->hi_pc = next_pos->hi_pc;
-            // Erase the next entry that wasn't needed
-            pos = m_aranges.erase (next_pos);
-            // Now recompute the end of the collection
-            end = m_aranges.end();
+            r1.hi_pc = r2.hi_pc;
+            indices.push_back(cursor);
         }
         else
-        {
-            // Two entries have either different offsets or there are gaps
-            // in the address range, move along, nothing to see here.
-            pos = next_pos;
-        }
+            merge = cursor;
     }
+
+    if (indices.empty())
+        return;
+
+    // Remove the merged ranges by shifting down all the keepers...
+    indices.push_back(m_aranges.size());
+    end = indices.size();
+    for (size_t target = indices[0], cursor = 1; cursor < end; ++cursor)
+    {
+        for (size_t i = indices[cursor - 1] + 1; i < indices[cursor]; ++i)
+            m_aranges[target++] = m_aranges[i];
+    }
+
+    // ...and drop the extra elements.
+    m_aranges.resize(m_aranges.size() - end + 1);
 }
 
 //----------------------------------------------------------------------