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

[Johnny Chen]

Hi Stephen,

Greg is skiing somewhere right now...
I modified your patch a tiny bit and will be committing it soon (it just passes the test suite).
Thanks for the patch!

Johnny

On Apr 15, 2011, at 6:40 AM, Stephen Wilson wrote:

> 
> 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);
> }
> 
> //----------------------------------------------------------------------
> 
> 
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits