dump_syms: use unordered_set<> instead of set<> for speed.

dump_syms: use unordered_set<> instead of set<> for speed.

dump_syms spends a lot of time trying to compare strings.
This change speeds up processing of libwebviewchromium.so by 30% on my linux
machine.

[uekawa]

Hi,

I don't know if C++11 features are allowed in breakpad yet, but this set<string>
is slow.

[Mark Mentovai]

tr1 isn’t necessarily available under that name in C++11, you would just
#include <unordered_set> and use std::unordered_set in that case. I’m afraid
that there’s no clean way to write this that works with both C++11 and pre-C++11
TR1.

However, on the Mac, although we build with C++11 language support, we don’t
build with the C++11 library. And on Linux, when C++11 is in use, the standard
library continues to provide <tr1/unordered_set>.

I guess we can try this and see if anything breaks.

https://breakpad.appspot.com/2714002/diff/1/common/dwarf_cu_to_module.cc
File common/dwarf_cu_to_module.cc (right):

https://breakpad.appspot.com/2714002/diff/1/common/dwarf_cu_to_module.cc#newc...
common/dwarf_cu_to_module.cc:50: #include <tr1/unordered_set>
Sort: tr1 < utility.

[uekawa]

This is my first patch to breakpad that I don't know how things really work.

Is there a try bot or is breakpad I'm-feeling-lucky-submit?
How does this get submitted ?

https://breakpad.appspot.com/2714002/diff/1/common/dwarf_cu_to_module.cc
File common/dwarf_cu_to_module.cc (right):

https://breakpad.appspot.com/2714002/diff/1/common/dwarf_cu_to_module.cc#newc...
common/dwarf_cu_to_module.cc:50: #include <tr1/unordered_set>
On 2014/06/25 13:24:29, Mark Mentovai wrote:
> Sort: tr1 < utility.

Done.

[Mark Mentovai]

LGTM and checked in as Breakpad r1341.

Breakpad doesn’t have dedicated “try” or buildbot infrastructure. It’s currently
being set up (bug 366164). In the mean time, the way to do this is to commit a
change to Breakpad and then change DEPS in Chrome to update Breakpad. You can
submit a Chrome try job for your DEPS update.

I noticed other uses of std::set in the DWARF code in src/common. Are any of
those hot spots too?

[uekawa]

On 2014/06/26 12:41:29, Mark Mentovai wrote:
> LGTM and checked in as Breakpad r1341.
> 
> Breakpad doesn’t have dedicated “try” or buildbot infrastructure. It’s
currently
> being set up (bug 366164). In the mean time, the way to do this is to commit a
> change to Breakpad and then change DEPS in Chrome to update Breakpad. You can
> submit a Chrome try job for your DEPS update.
> 
> I noticed other uses of std::set in the DWARF code in src/common. Are any of
> those hot spots too?

Now perf record / report looks much more sane with <5% on each symbol, so
there's less low-hanging fruit here.


Some random ideas:
 - remove the set<string> because we'll already have mapped the name in memory.
 - add a unordered_map<string /*mangled*/, string /*demangled*/> so that we
don't have to cxa_demangle every time.
 - malloc is called many times, use a different allocator
 - use unordered map for 
    std::map<unsigned long long,
google_breakpad::DwarfCUToModule::Specification>

[earthdok]

On 2014/06/25 13:24:28, Mark Mentovai wrote:
> tr1 isn’t necessarily available under that name in C++11, you would just
> #include <unordered_set> and use std::unordered_set in that case. I’m afraid
> that there’s no clean way to write this that works with both C++11 and
pre-C++11
> TR1.
> 
> However, on the Mac, although we build with C++11 language support, we don’t
> build with the C++11 library. And on Linux, when C++11 is in use, the standard
> library continues to provide <tr1/unordered_set>.

We actually use libc++ on Linux as well. The rationale is that tools like ASan
benefit from instrumenting not just user code but the C++ standard library as
well, which means rebuilding it from source. Libc++ is easier to build than
libstdc++ (and also has better support for some ASan features).

I'll send a patch to fix this, but as discussed at
https://code.google.com/p/chromium/issues/detail?id=391792 we should avoid
including tr1 where possible.