Page MenuHomePhabricator

[sanitizer] Add a fast version of StackDepotGet() for use in LSan. Add a class that holds a snapshot of the StackDepot optimized for querying by ID. This allows us to speed up LSan dramatically.
ClosedPublic

Authored by earthdok on Aug 23 2013, 3:11 PM.

Details

Reviewers
dvyukov
Summary

The naming is abominable all around. I'm open to suggestions. Maybe call
it simply StackDepotSnapshot?

Diff Detail

Event Timeline

dvyukov accepted this revision.Aug 24 2013, 4:05 AM

What performance improvement does it provide in your case?

lib/sanitizer_common/sanitizer_stackdepot.cc
225

PLease move this into template InternalBinarySearch().

lib/sanitizer_common/sanitizer_stackdepot.h
44

I would remove StackDepot prefix, since it's already in a class staring from StackDepot.

58

I think this should be static function in IdDescPair.

What performance improvement does it provide in your case?

Leak checking after a run of Chrome unit_tests used to take almost 3 minutes, now takes less than 10 seconds.

earthdok updated this revision to Unknown Object (????).Aug 26 2013, 3:25 AM
  • addressing dvyukov's suggestions

As a side note, I think we should stop prefixing everything with "Internal".

It really only makes sense when we're reimplementing a standard library function under the same name.

kcc added a comment.Aug 26 2013, 3:59 AM

I am ok with this change as a temporary measure, but long term we need to fix ld.so problem in lsan in some more efficient way, e.g.
by checking that the caller address in memalign belongs to ld.so.
Sergey, please make sure to explain the situation in detail in a bug report.

earthdok closed this revision.Dec 5 2014, 9:40 AM