[Lldb-commits] [PATCH] D70845: [lldb][NFC] Remove ThreadSafeSTLVector and ThreadSafeSTLMap and their use in ValueObjectSynthetic

[Raphael Isemann via Phabricator via lldb-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG315600f48005: [lldb][NFC] Remove ThreadSafeSTLVector and 
ThreadSafeSTLMap and their use in… (authored by teemperor).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70845

Files:
  lldb/include/lldb/Core/ThreadSafeSTLMap.h
  lldb/include/lldb/Core/ThreadSafeSTLVector.h
  lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
  lldb/source/Core/ValueObjectSyntheticFilter.cpp

Index: lldb/source/Core/ValueObjectSyntheticFilter.cpp
===
--- lldb/source/Core/ValueObjectSyntheticFilter.cpp
+++ lldb/source/Core/ValueObjectSyntheticFilter.cpp
@@ -48,8 +48,9 @@
 ValueObjectSynthetic::ValueObjectSynthetic(ValueObject ,
lldb::SyntheticChildrenSP filter)
 : ValueObject(parent), m_synth_sp(filter), m_children_byindex(),
-  m_name_toindex(), m_synthetic_children_count(UINT32_MAX),
-  m_synthetic_children_cache(), m_parent_type_name(parent.GetTypeName()),
+  m_name_toindex(), m_synthetic_children_cache(),
+  m_synthetic_children_count(UINT32_MAX),
+  m_parent_type_name(parent.GetTypeName()),
   m_might_have_children(eLazyBoolCalculate),
   m_provides_value(eLazyBoolCalculate) {
   SetName(parent.GetName());
@@ -177,14 +178,20 @@
   "filter said caches are stale - clearing",
   GetName().AsCString());
 // filter said that cached values are stale
-m_children_byindex.Clear();
-m_name_toindex.Clear();
+{
+  std::lock_guard guard(m_child_mutex);
+  m_children_byindex.clear();
+  m_name_toindex.clear();
+}
 // usually, an object's value can change but this does not alter its
 // children count for a synthetic VO that might indeed happen, so we need
 // to tell the upper echelons that they need to come back to us asking for
 // children
 m_children_count_valid = false;
-m_synthetic_children_cache.Clear();
+{
+  std::lock_guard guard(m_child_mutex);
+  m_synthetic_children_cache.clear();
+}
 m_synthetic_children_count = UINT32_MAX;
 m_might_have_children = eLazyBoolCalculate;
   } else {
@@ -232,7 +239,16 @@
   UpdateValueIfNeeded();
 
   ValueObject *valobj;
-  if (!m_children_byindex.GetValueForKey(idx, valobj)) {
+  bool child_is_cached;
+  {
+std::lock_guard guard(m_child_mutex);
+auto cached_child_it = m_children_byindex.find(idx);
+child_is_cached = cached_child_it != m_children_byindex.end();
+if (child_is_cached)
+  valobj = cached_child_it->second;
+  }
+
+  if (!child_is_cached) {
 if (can_create && m_synth_filter_up != nullptr) {
   LLDB_LOGF(log,
 "[ValueObjectSynthetic::GetChildAtIndex] name=%s, child at "
@@ -254,9 +270,12 @@
   if (!synth_guy)
 return synth_guy;
 
-  if (synth_guy->IsSyntheticChildrenGenerated())
-m_synthetic_children_cache.AppendObject(synth_guy);
-  m_children_byindex.SetValueForKey(idx, synth_guy.get());
+  {
+std::lock_guard guard(m_child_mutex);
+if (synth_guy->IsSyntheticChildrenGenerated())
+  m_synthetic_children_cache.push_back(synth_guy);
+m_children_byindex[idx] = synth_guy.get();
+  }
   synth_guy->SetPreferredDisplayLanguageIfNeeded(
   GetPreferredDisplayLanguage());
   return synth_guy;
@@ -297,13 +316,21 @@
   UpdateValueIfNeeded();
 
   uint32_t found_index = UINT32_MAX;
-  bool did_find = m_name_toindex.GetValueForKey(name.GetCString(), found_index);
+  bool did_find;
+  {
+std::lock_guard guard(m_child_mutex);
+auto name_to_index = m_name_toindex.find(name.GetCString());
+did_find = name_to_index != m_name_toindex.end();
+if (did_find)
+  found_index = name_to_index->second;
+  }
 
   if (!did_find && m_synth_filter_up != nullptr) {
 uint32_t index = m_synth_filter_up->GetIndexOfChildWithName(name);
 if (index == UINT32_MAX)
   return index;
-m_name_toindex.SetValueForKey(name.GetCString(), index);
+std::lock_guard guard(m_child_mutex);
+m_name_toindex[name.GetCString()] = index;
 return index;
   } else if (!did_find && m_synth_filter_up == nullptr)
 return UINT32_MAX;
Index: lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
===
--- lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
+++ lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
@@ -9,8 +9,6 @@
 #ifndef liblldb_ValueObjectSyntheticFilter_h_
 #define liblldb_ValueObjectSyntheticFilter_h_
 
-#include "lldb/Core/ThreadSafeSTLMap.h"
-#include "lldb/Core/ThreadSafeSTLVector.h"
 #include "lldb/Core/ValueObject.h"
 #include "lldb/Symbol/CompilerType.h"
 #include "lldb/Utility/ConstString.h"
@@ -135,19 +133,24 @@
   lldb::SyntheticChildrenSP m_synth_sp;
   std::unique_ptr 

[Lldb-commits] [PATCH] D70845: [lldb][NFC] Remove ThreadSafeSTLVector and ThreadSafeSTLMap and their use in ValueObjectSynthetic

[Pavel Labath via Phabricator via lldb-commits]

labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.

I'm yet to see a non-trivial algorithm, which can be made thread-safe by 
employing "thread safe" containers. +1 for deleting these and sorting the 
ValueObjectSynthetic issues later.


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D70845



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

[Lldb-commits] [PATCH] D70845: [lldb][NFC] Remove ThreadSafeSTLVector and ThreadSafeSTLMap and their use in ValueObjectSynthetic

[Raphael Isemann via Phabricator via lldb-commits]

teemperor created this revision.
teemperor added reviewers: labath, JDevlieghere, jingham.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

ThreadSafeSTLVector and ThreadSafeSTLMap are not useful for achieving any 
degree of thread safety in LLDB
and should be removed before they are used in more places. They are only used 
(unsurprisingly incorrectly) in
`ValueObjectSynthetic::GetChildAtIndex`, so this patch replaces their use there 
with a simple mutex with which
we guard the related data structures. This doesn't make 
ValueObjectSynthetic::GetChildAtIndex
any more thread-safe, but on the other hand it at least allows us to get rid of 
the ThreadSafeSTL* data structures
without changing the observable behaviour of ValueObjectSynthetic (beside that 
it is now a few bytes smaller).


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D70845

Files:
  lldb/include/lldb/Core/ThreadSafeSTLMap.h
  lldb/include/lldb/Core/ThreadSafeSTLVector.h
  lldb/include/lldb/Core/ValueObjectSyntheticFilter.h
  lldb/source/Core/ValueObjectSyntheticFilter.cpp

Index: lldb/source/Core/ValueObjectSyntheticFilter.cpp
===
--- lldb/source/Core/ValueObjectSyntheticFilter.cpp
+++ lldb/source/Core/ValueObjectSyntheticFilter.cpp
@@ -48,8 +48,9 @@
 ValueObjectSynthetic::ValueObjectSynthetic(ValueObject ,
lldb::SyntheticChildrenSP filter)
 : ValueObject(parent), m_synth_sp(filter), m_children_byindex(),
-  m_name_toindex(), m_synthetic_children_count(UINT32_MAX),
-  m_synthetic_children_cache(), m_parent_type_name(parent.GetTypeName()),
+  m_name_toindex(), m_synthetic_children_cache(),
+  m_synthetic_children_count(UINT32_MAX),
+  m_parent_type_name(parent.GetTypeName()),
   m_might_have_children(eLazyBoolCalculate),
   m_provides_value(eLazyBoolCalculate) {
   SetName(parent.GetName());
@@ -177,14 +178,20 @@
   "filter said caches are stale - clearing",
   GetName().AsCString());
 // filter said that cached values are stale
-m_children_byindex.Clear();
-m_name_toindex.Clear();
+{
+  std::lock_guard guard(m_child_mutex);
+  m_children_byindex.clear();
+  m_name_toindex.clear();
+}
 // usually, an object's value can change but this does not alter its
 // children count for a synthetic VO that might indeed happen, so we need
 // to tell the upper echelons that they need to come back to us asking for
 // children
 m_children_count_valid = false;
-m_synthetic_children_cache.Clear();
+{
+  std::lock_guard guard(m_child_mutex);
+  m_synthetic_children_cache.clear();
+}
 m_synthetic_children_count = UINT32_MAX;
 m_might_have_children = eLazyBoolCalculate;
   } else {
@@ -232,7 +239,16 @@
   UpdateValueIfNeeded();
 
   ValueObject *valobj;
-  if (!m_children_byindex.GetValueForKey(idx, valobj)) {
+  bool child_is_cached;
+  {
+std::lock_guard guard(m_child_mutex);
+auto cached_child_it = m_children_byindex.find(idx);
+child_is_cached = cached_child_it != m_children_byindex.end();
+if (child_is_cached)
+  valobj = cached_child_it->second;
+  }
+
+  if (!child_is_cached) {
 if (can_create && m_synth_filter_up != nullptr) {
   LLDB_LOGF(log,
 "[ValueObjectSynthetic::GetChildAtIndex] name=%s, child at "
@@ -254,9 +270,12 @@
   if (!synth_guy)
 return synth_guy;
 
-  if (synth_guy->IsSyntheticChildrenGenerated())
-m_synthetic_children_cache.AppendObject(synth_guy);
-  m_children_byindex.SetValueForKey(idx, synth_guy.get());
+  {
+std::lock_guard guard(m_child_mutex);
+if (synth_guy->IsSyntheticChildrenGenerated())
+  m_synthetic_children_cache.push_back(synth_guy);
+m_children_byindex[idx] = synth_guy.get();
+  }
   synth_guy->SetPreferredDisplayLanguageIfNeeded(
   GetPreferredDisplayLanguage());
   return synth_guy;
@@ -297,13 +316,21 @@
   UpdateValueIfNeeded();
 
   uint32_t found_index = UINT32_MAX;
-  bool did_find = m_name_toindex.GetValueForKey(name.GetCString(), found_index);
+  bool did_find;
+  {
+std::lock_guard guard(m_child_mutex);
+auto name_to_index = m_name_toindex.find(name.GetCString());
+did_find = name_to_index != m_name_toindex.end();
+if (did_find)
+  found_index = name_to_index->second;
+  }
 
   if (!did_find && m_synth_filter_up != nullptr) {
 uint32_t index = m_synth_filter_up->GetIndexOfChildWithName(name);
 if (index == UINT32_MAX)
   return index;
-m_name_toindex.SetValueForKey(name.GetCString(), index);
+std::lock_guard guard(m_child_mutex);
+m_name_toindex[name.GetCString()] = index;
 return index;
   } else if (!did_find && m_synth_filter_up == nullptr)
 return UINT32_MAX;
Index: lldb/include/lldb/Core/ValueObjectSyntheticFilter.h