[PATCH] D157459: Make DWARFContext more thread safe.

[Alex Langford via Phabricator via llvm-commits]

bulbazord added a comment.

In D157459#4596989 <https://reviews.llvm.org/D157459#4596989>, @ayermolo wrote:

> I am somewhat confused about how this is a performance regression. Doesn't sound like it currently working in multi threaded environment anyway. If it is implemented as is, I don't believe most tools that process debug information are multi threaded, and even if they are majority of time is spent actually parsing debug information.

I have not measured this change myself, so I can't definitely claim it is 100% a performance regression. However, there are 2 reasons I suspect this:

1. recursive_mutex is usually less performant because every time you try to acquire a lock with it, you need to do extra work to determine if the mutex is already acquired on the calling thread. How much slower, I cannot say as this is probably dependent on many factors (hardware, os, stdlib implementation, etc). I won't say not to use recursive_mutex because sometimes the use is completely justified and unavoidable. We should be aware of the penalty though.
2. One mutex locking the access to every member variable means that only one member can be parsed at a time. If one thread calls `DWARFContext::getDebugAbbrev` while another tries to parse an unrelated section, you're leaving performance on the table. Maybe that's not a priority or not the point, but if we're going to go this route we should be aware of the tradeoffs.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D157459/new/

https://reviews.llvm.org/D157459