jingham created this revision.
jingham added reviewers: JDevlieghere, jasonmolenda, kastiglione, labath.
Herald added a project: All.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.
The m_objects store in ClusterManager holds pointers to all the objects
managed by the ClusterManager. It is used to check whether
this ClusterManager already owns this pointer before adding it to
the ClusterManager's shared pointer or getting the shared pointer for the
object.
When the ClusterManager is deleted, we iterate over it to destroy all the
managed objects.
The most common operation by far is "GetSharedPointer" on the cluster.
With a SmallVector and llvm::is_contained the performance was non-linear
in the number of elements. For instance, printing all the elements of a 16M
element std::vector
didn't complete in the time I was willing to wait for it (hours).
Since we are mostly doing insert & contains, some kind of set is likely to be a
better data structure. In this patch I used SmallPtrSet. With
that, the same array prints out in 30 seconds. I couldn't see any noticeable
difference
for ValueObjects with a small number of children. I also tried a the same
patch using a
std::unordered_set but that was slightly slower and used a fair bit
more memory than the SmallPtrSet.
Other than performance, this is NFC.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D131996
Files:
lldb/include/lldb/Utility/SharedCluster.h
Index: lldb/include/lldb/Utility/SharedCluster.h
===================================================================
--- lldb/include/lldb/Utility/SharedCluster.h
+++ lldb/include/lldb/Utility/SharedCluster.h
@@ -11,7 +11,7 @@
#include "lldb/Utility/LLDBAssert.h"
#include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/SmallPtrSet.h"
#include <memory>
#include <mutex>
@@ -32,15 +32,15 @@
void ManageObject(T *new_object) {
std::lock_guard<std::mutex> guard(m_mutex);
- assert(!llvm::is_contained(m_objects, new_object) &&
- "ManageObject called twice for the same object?");
- m_objects.push_back(new_object);
+ auto ret = m_objects.insert(new_object);
+ assert(ret.second && "ManageObject called twice for the same object?");
}
std::shared_ptr<T> GetSharedPointer(T *desired_object) {
std::lock_guard<std::mutex> guard(m_mutex);
auto this_sp = this->shared_from_this();
- if (!llvm::is_contained(m_objects, desired_object)) {
+ size_t count = m_objects.count(desired_object);
+ if (count == 0) {
lldbassert(false && "object not found in shared cluster when expected");
desired_object = nullptr;
}
@@ -49,8 +49,7 @@
private:
ClusterManager() : m_objects() {}
-
- llvm::SmallVector<T *, 16> m_objects;
+ llvm::SmallPtrSet<T *, 16> m_objects;
std::mutex m_mutex;
};
Index: lldb/include/lldb/Utility/SharedCluster.h
===================================================================
--- lldb/include/lldb/Utility/SharedCluster.h
+++ lldb/include/lldb/Utility/SharedCluster.h
@@ -11,7 +11,7 @@
#include "lldb/Utility/LLDBAssert.h"
#include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/SmallPtrSet.h"
#include <memory>
#include <mutex>
@@ -32,15 +32,15 @@
void ManageObject(T *new_object) {
std::lock_guard<std::mutex> guard(m_mutex);
- assert(!llvm::is_contained(m_objects, new_object) &&
- "ManageObject called twice for the same object?");
- m_objects.push_back(new_object);
+ auto ret = m_objects.insert(new_object);
+ assert(ret.second && "ManageObject called twice for the same object?");
}
std::shared_ptr<T> GetSharedPointer(T *desired_object) {
std::lock_guard<std::mutex> guard(m_mutex);
auto this_sp = this->shared_from_this();
- if (!llvm::is_contained(m_objects, desired_object)) {
+ size_t count = m_objects.count(desired_object);
+ if (count == 0) {
lldbassert(false && "object not found in shared cluster when expected");
desired_object = nullptr;
}
@@ -49,8 +49,7 @@
private:
ClusterManager() : m_objects() {}
-
- llvm::SmallVector<T *, 16> m_objects;
+ llvm::SmallPtrSet<T *, 16> m_objects;
std::mutex m_mutex;
};
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
