[PATCH] D157459: Make DWARFContext more thread safe.
Greg Clayton via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Aug 9 12:02:20 PDT 2023
clayborg added a comment.
In D157459#4573865 <https://reviews.llvm.org/D157459#4573865>, @dblaikie wrote:
> Not sure how I feel about adding multithreading at this level, versus in a wrapper API for those that need it? (insert an interface (`DWARFContext`, then rename the current concrete implementation as `DWARFContextImpl`) then have a `ThreadSafeDWARFContext` that wraps (either as a template, or with a `unique_ptr<DWARFContext>` member) another `DWARFContext` and adds the locking) Though it's probably not the end of the world to add it here directly.
I am fine with either way. The main issue about having a class that wraps a DWARFContext is internally the DWARFDie and DWARFUnit and possibly others, have access to their DWARFContext via the DWARFUnit and they can call some of these APIs. So wrapping won't really help for these important cases. Seems cleaner to me to have the mutex live in this class, but I am open to everyone's comments to figure out what is best for the codebase.
> Can probably use Class Template Argument Deduction when locking, like this:
>
> std::unique_lock LockGuard(Mutex);
Good idea, I fixed all locations as suggested.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D157459/new/
https://reviews.llvm.org/D157459
More information about the llvm-commits
mailing list