[PATCH] D157459: Make DWARFContext more thread safe.

[Alex Langford via Phabricator via llvm-commits]

bulbazord 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?

This approach works for me. I know it is more complex than what exists today so I appreciate you taking the time to address our concerns.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157459