[PATCH] D157459: Make DWARFContext more thread safe.

[Jonas Devlieghere via Phabricator via llvm-commits]

JDevlieghere added a comment.

In D157459#4612553 <https://reviews.llvm.org/D157459#4612553>, @clayborg wrote:

> Ok, I wanted to try and protect just the call to "const DWARFUnitIndex &DWARFContext::getCUIndex()" using examples people have mentioned and I came up with this solution to see if anyone likes it:
>
> In DWARFContext.h:
>
>   class DWARFContextStateImpl {
>   public:
>     virtual ~DWARFContextStateImpl() = default;
>     virtual const DWARFUnitIndex &getCUIndex(DWARFContext &D) = 0;
>   };
>   
>   class DWARFContextStateSingleThreaded : public DWARFContextStateImpl {
>     std::unique_ptr<DWARFUnitIndex> CUIndex;
>   public:
>     const DWARFUnitIndex &getCUIndex(DWARFContext &D) override;
>   };
>   
>   class DWARFContextStateMultiThreaded : public DWARFContextStateSingleThreaded {
>     std::recursive_mutex Mutex;
>   public:
>     const DWARFUnitIndex &getCUIndex(DWARFContext &D) override;
>   };
>   
>   // DWARFContext member variables
>   ...
>   // std::unique_ptr<DWARFUnitIndex> CUIndex; // This was the old ivar which is now contained in a subclass of DWARFContextStateImpl
>   std::unique_ptr<DWARFContextStateImpl> State;
>   ...
>
> Then to implement this we can do the following in the DWARContext.cpp file:
>
>   const DWARFUnitIndex &DWARFContext::getCUIndex() {
>     return State->getCUIndex(*this);
>   }
>   
>   const DWARFUnitIndex &
>   DWARFContext::DWARFContextStateSingleThreaded::getCUIndex(DWARFContext &D) {
>     if (CUIndex)
>       return *CUIndex;
>   
>     DataExtractor CUIndexData(D.DObj->getCUIndexSection(), D.isLittleEndian(), 0);
>     CUIndex = std::make_unique<DWARFUnitIndex>(DW_SECT_INFO);
>     bool IsParseSuccessful = CUIndex->parse(CUIndexData);
>     if (IsParseSuccessful)
>       fixupIndex(D, *CUIndex);
>     return *CUIndex;
>   }
>   
>   const DWARFUnitIndex &
>   DWARFContext::DWARFContextStateMultiThreaded::getCUIndex(DWARFContext &D) {
>     std::unique_lock LockGuard(Mutex);
>     return DWARFContextStateSingleThreaded::getCUIndex(D);
>   }
>
> Then if we enable threading with a bool in the constructor:
>
>   DWARFContext::DWARFContext(..., bool EnableMultiThreading) {
>     if (EnableMultiThreading)
>       State.reset(new DWARFContextStateMultiThreaded());
>     else
>       State.reset(new DWARFContextStateSingleThreaded());
>   }
>
> If we like this idea, I can add all member variables that we want the thread protect into DWARFContextStateSingleThreaded and implement the wrappers for each accessor. Does this kind of solution work for anyone?

I like this approach 👍


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157459