[Lldb-commits] [lldb] r105834 - in /lldb/trunk: include/lldb/Symbol/Symtab.h source/Symbol/Symtab.cpp

Greg Clayton gclayton at apple.com
Wed Jun 16 09:18:55 PDT 2010


We will need to use the scoped timers to make sure that performance doesn't degrade on MacOSX. File size, I don't much care about, but I do care about performance. And the last time I checked the performance, there was a 3x performance difference (qsort being 3x faster than std::sort (with gcc 4.2)). Symbols tables are high up on the list when it comes to things we need to keep fast.

Greg Clayton


On Jun 15, 2010, at 3:25 PM, Owen Anderson wrote:

> Ping?
> 
> On Jun 11, 2010, at 2:40 PM, Owen Anderson wrote:
> 
>> OK, so I did some measurement on this issue, using optimized (-Os) builds of LLDB, without debug symbols, targeting x86-64.
>> 
>> When using qsort_r, Symtab.o is 43KB.
>> When using std::sort, Symtab.o is 46KB.
>> A 7% increase in file size on a single file.
>> 
>> The qsort_r version of LLDB.framework is 32,840,512 bytes.
>> The std::sort version of LLDB.framework is 32,840,937 bytes.
>> A 0.001% increase in file size of the overall library.  :-)
>> 
>> Interestingly, it appears that those extra symbols get coalesced with ones already present (presumably in the LLVM libraries), so it doesn't actually add much at all to the resulting library.
>> 
>> --Owen
>> 
>> On Jun 11, 2010, at 2:03 PM, Jason Molenda wrote:
>> 
>>> Please back this change out Owen, until you've addressed the point Greg made here -
>>> 	http://lists.cs.uiuc.edu/pipermail/lldb-dev/2010-June/000039.html
>>> 
>>> J
>>> 
>>> On Jun 11, 2010, at 1:52 PM, Owen Anderson wrote:
>>> 
>>>> Author: resistor
>>>> Date: Fri Jun 11 15:52:57 2010
>>>> New Revision: 105834
>>>> 
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=105834&view=rev
>>>> Log:
>>>> Replace qsort_r with std::sort.  This gets rid of a lot of portability
>>>> ickiness, and is cleaner to boot.
>>>> 
>>>> I'm fairly confident that I converted the comparator over properly,
>>>> and what testing I could figure out how to run seemed to pass, but it
>>>> would be great if someone in the know could check behind me.
>>>> 
>>>> Modified:
>>>> lldb/trunk/include/lldb/Symbol/Symtab.h
>>>> lldb/trunk/source/Symbol/Symtab.cpp
>>>> 
>>>> Modified: lldb/trunk/include/lldb/Symbol/Symtab.h
>>>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/Symtab.h?rev=105834&r1=105833&r2=105834&view=diff
>>>> ==============================================================================
>>>> --- lldb/trunk/include/lldb/Symbol/Symtab.h (original)
>>>> +++ lldb/trunk/include/lldb/Symbol/Symtab.h Fri Jun 11 15:52:57 2010
>>>> @@ -59,8 +59,6 @@
>>>>  typedef collection::iterator        iterator;
>>>>  typedef collection::const_iterator  const_iterator;
>>>> 
>>>> -    static  int         CompareSymbolValueByIndex (void *thunk, const void *a, const void *b);
>>>> -    static  int         CompareSymbolValueByIndexLinux (const void *a, const void *b, void *thunk);
>>>>          void        InitNameIndexes ();
>>>>          void        InitAddressIndexes ();
>>>> 
>>>> 
>>>> Modified: lldb/trunk/source/Symbol/Symtab.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/Symtab.cpp?rev=105834&r1=105833&r2=105834&view=diff
>>>> ==============================================================================
>>>> --- lldb/trunk/source/Symbol/Symtab.cpp (original)
>>>> +++ lldb/trunk/source/Symbol/Symtab.cpp Fri Jun 11 15:52:57 2010
>>>> @@ -214,46 +214,36 @@
>>>>  const Symbol *symbols;
>>>> };
>>>> 
>>>> -int
>>>> -Symtab::CompareSymbolValueByIndex (void *thunk, const void *a, const void *b)
>>>> -{
>>>> -    const Symbol *symbols = (const Symbol *)thunk;
>>>> -    uint32_t index_a = *((uint32_t *) a);
>>>> -    uint32_t index_b = *((uint32_t *) b);
>>>> -
>>>> -    addr_t value_a;
>>>> -    addr_t value_b;
>>>> -    if (symbols[index_a].GetValue().GetSection() == symbols[index_b].GetValue().GetSection())
>>>> -    {
>>>> -        value_a = symbols[index_a].GetValue ().GetOffset();
>>>> -        value_b = symbols[index_b].GetValue ().GetOffset();
>>>> -    }
>>>> -    else
>>>> -    {
>>>> -        value_a = symbols[index_a].GetValue ().GetFileAddress();
>>>> -        value_b = symbols[index_b].GetValue ().GetFileAddress();
>>>> -    }
>>>> -
>>>> -    if (value_a == value_b)
>>>> -    {
>>>> -        // The if the values are equal, use the original symbol user ID
>>>> -        lldb::user_id_t uid_a = symbols[index_a].GetID();
>>>> -        lldb::user_id_t uid_b = symbols[index_b].GetID();
>>>> -        if (uid_a < uid_b)
>>>> -            return -1;
>>>> -        if (uid_a > uid_b)
>>>> -            return 1;
>>>> -        return 0;
>>>> -    }
>>>> -    else if (value_a < value_b)
>>>> -        return -1;
>>>> -
>>>> -    return 1;
>>>> -}
>>>> +namespace {
>>>> +    struct SymbolIndexComparator {
>>>> +        const std::vector<Symbol>& symbols;
>>>> +        SymbolIndexComparator(const std::vector<Symbol>& s) : symbols(s) { }
>>>> +        bool operator()(uint32_t index_a, uint32_t index_b) {
>>>> +            addr_t value_a;
>>>> +            addr_t value_b;
>>>> +            if (symbols[index_a].GetValue().GetSection() == symbols[index_b].GetValue().GetSection()) {
>>>> +                value_a = symbols[index_a].GetValue ().GetOffset();
>>>> +                value_b = symbols[index_b].GetValue ().GetOffset();
>>>> +            } else {
>>>> +                value_a = symbols[index_a].GetValue ().GetFileAddress();
>>>> +                value_b = symbols[index_b].GetValue ().GetFileAddress();
>>>> +            }
>>>> 
>>>> -int Symtab::CompareSymbolValueByIndexLinux(const void* a, const void* b, void* thunk) 
>>>> -{
>>>> -    return CompareSymbolValueByIndex(thunk, a, b);
>>>> +            if (value_a == value_b) {
>>>> +                // The if the values are equal, use the original symbol user ID
>>>> +                lldb::user_id_t uid_a = symbols[index_a].GetID();
>>>> +                lldb::user_id_t uid_b = symbols[index_b].GetID();
>>>> +                if (uid_a < uid_b)
>>>> +                    return true;
>>>> +                if (uid_a > uid_b)
>>>> +                    return false;
>>>> +                return false;
>>>> +            } else if (value_a < value_b)
>>>> +                return true;
>>>> +        
>>>> +            return false;
>>>> +        }
>>>> +    };
>>>> }
>>>> 
>>>> void
>>>> @@ -263,13 +253,8 @@
>>>>  if (indexes.size() <= 1)
>>>>      return;
>>>> 
>>>> -    // Sort the indexes in place using qsort
>>>> -    // FIXME: (WRONGDEFINE) Need a better define for this! 
>>>> -#ifdef __APPLE__
>>>> -    ::qsort_r (&indexes[0], indexes.size(), sizeof(uint32_t), (void *)&m_symbols[0], Symtab::CompareSymbolValueByIndex);
>>>> -#else
>>>> -    ::qsort_r (&indexes[0], indexes.size(), sizeof(uint32_t), CompareSymbolValueByIndexLinux, (void *)&m_symbols[0]);
>>>> -#endif
>>>> +    // Sort the indexes in place using std::sort
>>>> +    std::sort(indexes.begin(), indexes.end(), SymbolIndexComparator(m_symbols));
>>>> 
>>>>  // Remove any duplicates if requested
>>>>  if (remove_duplicates)
>>>> 
>>>> 
>>>> _______________________________________________
>>>> lldb-commits mailing list
>>>> lldb-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
>>> 
>> 
>> _______________________________________________
>> lldb-commits mailing list
>> lldb-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
> 
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits





More information about the lldb-commits mailing list