Author: Raphael Isemann Date: 2019-12-03T09:18:44+01:00 New Revision: 315600f480055f5143aaa245f25bd25221edfa91
URL: https://github.com/llvm/llvm-project/commit/315600f480055f5143aaa245f25bd25221edfa91 DIFF: https://github.com/llvm/llvm-project/commit/315600f480055f5143aaa245f25bd25221edfa91.diff LOG: [lldb][NFC] Remove ThreadSafeSTLVector and ThreadSafeSTLMap and their use in ValueObjectSynthetic Summary: 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). Reviewers: labath, JDevlieghere, jingham Reviewed By: labath Subscribers: lldb-commits Tags: #lldb Differential Revision: https://reviews.llvm.org/D70845 Added: Modified: lldb/include/lldb/Core/ValueObjectSyntheticFilter.h lldb/source/Core/ValueObjectSyntheticFilter.cpp Removed: lldb/include/lldb/Core/ThreadSafeSTLMap.h lldb/include/lldb/Core/ThreadSafeSTLVector.h ################################################################################ diff --git a/lldb/include/lldb/Core/ThreadSafeSTLMap.h b/lldb/include/lldb/Core/ThreadSafeSTLMap.h deleted file mode 100644 index df0208cd49b3..000000000000 --- a/lldb/include/lldb/Core/ThreadSafeSTLMap.h +++ /dev/null @@ -1,128 +0,0 @@ -//===-- ThreadSafeSTLMap.h --------------------------------------*- C++ -*-===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// - -#ifndef liblldb_ThreadSafeSTLMap_h_ -#define liblldb_ThreadSafeSTLMap_h_ - -#include <map> -#include <mutex> - -#include "lldb/lldb-defines.h" - -namespace lldb_private { - -template <typename _Key, typename _Tp> class ThreadSafeSTLMap { -public: - typedef std::map<_Key, _Tp> collection; - typedef typename collection::iterator iterator; - typedef typename collection::const_iterator const_iterator; - // Constructors and Destructors - ThreadSafeSTLMap() : m_collection(), m_mutex() {} - - ~ThreadSafeSTLMap() {} - - bool IsEmpty() const { - std::lock_guard<std::recursive_mutex> guard(m_mutex); - return m_collection.empty(); - } - - void Clear() { - std::lock_guard<std::recursive_mutex> guard(m_mutex); - return m_collection.clear(); - } - - size_t Erase(const _Key &key) { - std::lock_guard<std::recursive_mutex> guard(m_mutex); - return EraseNoLock(key); - } - - size_t EraseNoLock(const _Key &key) { return m_collection.erase(key); } - - bool GetValueForKey(const _Key &key, _Tp &value) const { - std::lock_guard<std::recursive_mutex> guard(m_mutex); - return GetValueForKeyNoLock(key, value); - } - - // Call this if you have already manually locked the mutex using the - // GetMutex() accessor - bool GetValueForKeyNoLock(const _Key &key, _Tp &value) const { - const_iterator pos = m_collection.find(key); - if (pos != m_collection.end()) { - value = pos->second; - return true; - } - return false; - } - - bool GetFirstKeyForValue(const _Tp &value, _Key &key) const { - std::lock_guard<std::recursive_mutex> guard(m_mutex); - return GetFirstKeyForValueNoLock(value, key); - } - - bool GetFirstKeyForValueNoLock(const _Tp &value, _Key &key) const { - const_iterator pos, end = m_collection.end(); - for (pos = m_collection.begin(); pos != end; ++pos) { - if (pos->second == value) { - key = pos->first; - return true; - } - } - return false; - } - - bool LowerBound(const _Key &key, _Key &match_key, _Tp &match_value, - bool decrement_if_not_equal) const { - std::lock_guard<std::recursive_mutex> guard(m_mutex); - return LowerBoundNoLock(key, match_key, match_value, - decrement_if_not_equal); - } - - bool LowerBoundNoLock(const _Key &key, _Key &match_key, _Tp &match_value, - bool decrement_if_not_equal) const { - const_iterator pos = m_collection.lower_bound(key); - if (pos != m_collection.end()) { - match_key = pos->first; - if (decrement_if_not_equal && key != match_key && - pos != m_collection.begin()) { - --pos; - match_key = pos->first; - } - match_value = pos->second; - return true; - } - return false; - } - - iterator lower_bound_unsafe(const _Key &key) { - return m_collection.lower_bound(key); - } - - void SetValueForKey(const _Key &key, const _Tp &value) { - std::lock_guard<std::recursive_mutex> guard(m_mutex); - SetValueForKeyNoLock(key, value); - } - - // Call this if you have already manually locked the mutex using the - // GetMutex() accessor - void SetValueForKeyNoLock(const _Key &key, const _Tp &value) { - m_collection[key] = value; - } - - std::recursive_mutex &GetMutex() { return m_mutex; } - -private: - collection m_collection; - mutable std::recursive_mutex m_mutex; - - // For ThreadSafeSTLMap only - DISALLOW_COPY_AND_ASSIGN(ThreadSafeSTLMap); -}; - -} // namespace lldb_private - -#endif // liblldb_ThreadSafeSTLMap_h_ diff --git a/lldb/include/lldb/Core/ThreadSafeSTLVector.h b/lldb/include/lldb/Core/ThreadSafeSTLVector.h deleted file mode 100644 index e1666a69ef7e..000000000000 --- a/lldb/include/lldb/Core/ThreadSafeSTLVector.h +++ /dev/null @@ -1,72 +0,0 @@ -//===-- ThreadSafeSTLVector.h ------------------------------------*- C++ -//-*-===// -// -// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. -// See https://llvm.org/LICENSE.txt for license information. -// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception -// -//===----------------------------------------------------------------------===// - -#ifndef liblldb_ThreadSafeSTLVector_h_ -#define liblldb_ThreadSafeSTLVector_h_ - -#include <mutex> -#include <vector> - -#include "lldb/lldb-defines.h" - -namespace lldb_private { - -template <typename _Object> class ThreadSafeSTLVector { -public: - typedef std::vector<_Object> collection; - typedef typename collection::iterator iterator; - typedef typename collection::const_iterator const_iterator; - // Constructors and Destructors - ThreadSafeSTLVector() : m_collection(), m_mutex() {} - - ~ThreadSafeSTLVector() = default; - - bool IsEmpty() const { - std::lock_guard<std::recursive_mutex> guard(m_mutex); - return m_collection.empty(); - } - - void Clear() { - std::lock_guard<std::recursive_mutex> guard(m_mutex); - return m_collection.clear(); - } - - size_t GetCount() { - std::lock_guard<std::recursive_mutex> guard(m_mutex); - return m_collection.size(); - } - - void AppendObject(_Object &object) { - std::lock_guard<std::recursive_mutex> guard(m_mutex); - m_collection.push_back(object); - } - - _Object GetObject(size_t index) { - std::lock_guard<std::recursive_mutex> guard(m_mutex); - return m_collection.at(index); - } - - void SetObject(size_t index, const _Object &object) { - std::lock_guard<std::recursive_mutex> guard(m_mutex); - m_collection.at(index) = object; - } - - std::recursive_mutex &GetMutex() { return m_mutex; } - -private: - collection m_collection; - mutable std::recursive_mutex m_mutex; - - // For ThreadSafeSTLVector only - DISALLOW_COPY_AND_ASSIGN(ThreadSafeSTLVector); -}; - -} // namespace lldb_private - -#endif // liblldb_ThreadSafeSTLVector_h_ diff --git a/lldb/include/lldb/Core/ValueObjectSyntheticFilter.h b/lldb/include/lldb/Core/ValueObjectSyntheticFilter.h index 3b14a3e9f388..ec395095351d 100644 --- a/lldb/include/lldb/Core/ValueObjectSyntheticFilter.h +++ b/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 @@ class ValueObjectSynthetic : public ValueObject { lldb::SyntheticChildrenSP m_synth_sp; std::unique_ptr<SyntheticChildrenFrontEnd> m_synth_filter_up; - typedef ThreadSafeSTLMap<uint32_t, ValueObject *> ByIndexMap; - typedef ThreadSafeSTLMap<const char *, uint32_t> NameToIndexMap; - typedef ThreadSafeSTLVector<lldb::ValueObjectSP> SyntheticChildrenCache; + typedef std::map<uint32_t, ValueObject *> ByIndexMap; + typedef std::map<const char *, uint32_t> NameToIndexMap; + typedef std::vector<lldb::ValueObjectSP> SyntheticChildrenCache; typedef ByIndexMap::iterator ByIndexIterator; typedef NameToIndexMap::iterator NameToIndexIterator; + std::mutex m_child_mutex; + /// Guarded by m_child_mutex; ByIndexMap m_children_byindex; + /// Guarded by m_child_mutex; NameToIndexMap m_name_toindex; + /// Guarded by m_child_mutex; + SyntheticChildrenCache m_synthetic_children_cache; + uint32_t m_synthetic_children_count; // FIXME use the ValueObject's // ChildrenManager instead of a special // purpose solution - SyntheticChildrenCache m_synthetic_children_cache; ConstString m_parent_type_name; diff --git a/lldb/source/Core/ValueObjectSyntheticFilter.cpp b/lldb/source/Core/ValueObjectSyntheticFilter.cpp index a6bf35eac70a..a30be1b08338 100644 --- a/lldb/source/Core/ValueObjectSyntheticFilter.cpp +++ b/lldb/source/Core/ValueObjectSyntheticFilter.cpp @@ -48,8 +48,9 @@ class DummySyntheticFrontEnd : public SyntheticChildrenFrontEnd { ValueObjectSynthetic::ValueObjectSynthetic(ValueObject &parent, 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 @@ bool ValueObjectSynthetic::UpdateValue() { "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<std::mutex> 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<std::mutex> 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 @@ lldb::ValueObjectSP ValueObjectSynthetic::GetChildAtIndex(size_t idx, UpdateValueIfNeeded(); ValueObject *valobj; - if (!m_children_byindex.GetValueForKey(idx, valobj)) { + bool child_is_cached; + { + std::lock_guard<std::mutex> 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 @@ lldb::ValueObjectSP ValueObjectSynthetic::GetChildAtIndex(size_t idx, 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<std::mutex> 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 @@ size_t ValueObjectSynthetic::GetIndexOfChildWithName(ConstString name) { UpdateValueIfNeeded(); uint32_t found_index = UINT32_MAX; - bool did_find = m_name_toindex.GetValueForKey(name.GetCString(), found_index); + bool did_find; + { + std::lock_guard<std::mutex> 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<std::mutex> guard(m_child_mutex); + m_name_toindex[name.GetCString()] = index; return index; } else if (!did_find && m_synth_filter_up == nullptr) return UINT32_MAX; _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
