[Lldb-commits] [PATCH] D70845: [lldb][NFC] Remove ThreadSafeSTLVector and ThreadSafeSTLMap and their use in ValueObjectSynthetic
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
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
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