[PATCH] D157459: Make DWARFContext more thread safe.

[Greg Clayton via Phabricator via llvm-commits]

clayborg added a comment.

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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157459