[Lldb-commits] [PATCH] D49968: DWZ 02/06: Refactor DWARFUnit::ExtractDIEsScoped

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil created this revision.
jankratochvil added a reviewer: clayborg.
Herald added subscribers: JDevlieghere, aprantl.

Change the method of construction from:

  DWARFUnit::ScopedExtractDIEs DWARFUnit::ExtractDIEsScoped()

to:

  DWARFUnit::ScopedExtractDIEs::ScopedExtractDIEs(DWARFUnit *cu)

So that a later patch can implement:

  std::unordered_map<const DWARFUnit *, DWARFUnit::ScopedExtractDIEs>
      .emplace(DWARFUnit *,DWARFUnit *)

so that if the map element already exists `DWARFUnit::ScopedExtractDIEs` is not 
temporarily constructed. If the caller called `DWARFUnit::ExtractDIEsScoped()` 
as an `emplace` parameter it would get constructed already in the caller.
Patch also contains what I considered as a general cleanup.


https://reviews.llvm.org/D49968

Files:
  source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
  source/Plugins/SymbolFile/DWARF/DWARFUnit.h
  source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp


Index: source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -55,7 +55,8 @@
   };
 
   auto extract_fn = [&units_to_index, &clear_cu_dies](size_t cu_idx) {
-    clear_cu_dies[cu_idx] = units_to_index[cu_idx]->ExtractDIEsScoped();
+    clear_cu_dies[cu_idx] =
+        DWARFUnit::ScopedExtractDIEs(units_to_index[cu_idx]);
   };
 
   // Create a task runner that extracts dies for each DWARF compile unit in a
Index: source/Plugins/SymbolFile/DWARF/DWARFUnit.h
===================================================================
--- source/Plugins/SymbolFile/DWARF/DWARFUnit.h
+++ source/Plugins/SymbolFile/DWARF/DWARFUnit.h
@@ -43,16 +43,20 @@
   void ExtractDIEsIfNeeded();
 
   class ScopedExtractDIEs {
-    DWARFUnit *m_cu;
   public:
-    bool m_clear_dies = false;
     ScopedExtractDIEs(DWARFUnit *cu);
+    void ClearDIEs() { assert(!m_clear_dies); m_clear_dies = true; }
+    // Do not touch m_cu anymore.
+    void Disable() { m_unlock = false; m_clear_dies = false; }
     ~ScopedExtractDIEs();
-    DISALLOW_COPY_AND_ASSIGN(ScopedExtractDIEs);
     ScopedExtractDIEs(ScopedExtractDIEs &&rhs);
     ScopedExtractDIEs &operator=(ScopedExtractDIEs &&rhs);
+  private:
+    DWARFUnit *m_cu;
+    bool m_clear_dies = false;
+    bool m_unlock;
+    DISALLOW_COPY_AND_ASSIGN(ScopedExtractDIEs);
   };
-  ScopedExtractDIEs ExtractDIEsScoped();
 
   DWARFDIE LookupAddress(const dw_addr_t address);
   size_t AppendDIEsWithTag(const dw_tag_t tag,
@@ -227,6 +231,7 @@
   void ParseProducerInfo();
   void ExtractDIEsRWLocked();
   void ClearDIEsRWLocked();
+  void ExtractDIEsScoped(ScopedExtractDIEs &scoped);
 
   // Get the DWARF unit DWARF debug informration entry. Parse the single DIE
   // if needed.
Index: source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
+++ source/Plugins/SymbolFile/DWARF/DWARFUnit.cpp
@@ -98,35 +98,30 @@
 // and no ExtractDIEsIfNeeded() has been executed during this ScopedExtractDIEs
 // lifetime.
 //----------------------------------------------------------------------
-DWARFUnit::ScopedExtractDIEs DWARFUnit::ExtractDIEsScoped() {
-  ScopedExtractDIEs scoped(this);
-
+void DWARFUnit::ExtractDIEsScoped(ScopedExtractDIEs &scoped) {
   {
     llvm::sys::ScopedReader lock(m_die_array_mutex);
     if (!m_die_array.empty())
-      return scoped; // Already parsed
+      return; // Already parsed
   }
   llvm::sys::ScopedWriter lock(m_die_array_mutex);
   if (!m_die_array.empty())
-    return scoped; // Already parsed
-
-  // Otherwise m_die_array would be already populated.
-  lldbassert(!m_cancel_scopes);
+    return; // Already parsed
 
   ExtractDIEsRWLocked();
-  scoped.m_clear_dies = true;
-  return scoped;
+  scoped.ClearDIEs();
 }
 
 DWARFUnit::ScopedExtractDIEs::ScopedExtractDIEs(DWARFUnit *cu) : m_cu(cu) {
   lldbassert(m_cu);
   m_cu->m_die_array_scoped_mutex.lock_shared();
+  m_unlock = true;
+  m_cu->ExtractDIEsScoped(*this);
 }
 
 DWARFUnit::ScopedExtractDIEs::~ScopedExtractDIEs() {
-  if (!m_cu)
-    return;
-  m_cu->m_die_array_scoped_mutex.unlock_shared();
+  if (m_unlock)
+    m_cu->m_die_array_scoped_mutex.unlock_shared();
   if (!m_clear_dies || m_cu->m_cancel_scopes)
     return;
   // Be sure no other ScopedExtractDIEs is running anymore.
@@ -139,14 +134,14 @@
 
 DWARFUnit::ScopedExtractDIEs::ScopedExtractDIEs(ScopedExtractDIEs &&rhs)
     : m_cu(rhs.m_cu), m_clear_dies(rhs.m_clear_dies) {
-  rhs.m_cu = nullptr;
+  Disable();
 }
 
 DWARFUnit::ScopedExtractDIEs &DWARFUnit::ScopedExtractDIEs::operator=(
     DWARFUnit::ScopedExtractDIEs &&rhs) {
   m_cu = rhs.m_cu;
-  rhs.m_cu = nullptr;
   m_clear_dies = rhs.m_clear_dies;
+  rhs.Disable();
   return *this;
 }
 
@@ -447,7 +442,7 @@
   // If the DIEs weren't parsed, then we don't want all dies for all compile
   // units to stay loaded when they weren't needed. So we can end up parsing
   // the DWARF and then throwing them all away to keep memory usage down.
-  ScopedExtractDIEs clear_dies(ExtractDIEsScoped());
+  ScopedExtractDIEs clear_dies(this);
 
   die = DIEPtr();
   if (die)

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits