Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(3099)

Unified Diff: base/memory/weak_ptr.cc

Issue 2963623002: Make base::WeakPtr::Get() fast (Closed)
Patch Set: Move WeakReference::Flag::Invalidate out-of-line; it's only called from WeakReferenceOwner::Invalid… Created 3 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: base/memory/weak_ptr.cc
diff --git a/base/memory/weak_ptr.cc b/base/memory/weak_ptr.cc
index 8879651e6da7bfca1e4ce605c74e62bb69e259af..1e9403e71fbec0baa770c18fb12bb815a09ab7e5 100644
--- a/base/memory/weak_ptr.cc
+++ b/base/memory/weak_ptr.cc
@@ -4,50 +4,71 @@
#include "base/memory/weak_ptr.h"
+#include "base/debug/leak_annotations.h"
+
namespace base {
namespace internal {
-WeakReference::Flag::Flag() : is_valid_(true) {
+static constexpr uintptr_t kTrueMask = ~static_cast<uintptr_t>(0);
gab 2017/07/18 20:31:08 rm "static": it's redundant at file-scope level
hans 2017/07/19 08:08:23 Done.
+
+WeakReference::Flag::Flag() : is_valid_(kTrueMask) {
+#if DCHECK_IS_ON()
// Flags only become bound when checked for validity, or invalidated,
// so that we can check that later validity/invalidation operations on
// the same Flag take place on the same sequenced thread.
sequence_checker_.DetachFromSequence();
+#endif
+}
+
+WeakReference::Flag::Flag(WeakReference::Flag::NullFlagTag) : is_valid_(false) {
+ // There is no need for sequence_checker_.DetachFromSequence() because the
+ // null flag doesn't participate in the sequence checks. See DCHECK in
+ // Invalidate() and IsValid().
+
+ // Keep the object alive perpetually, even when there are no references to it.
+ AddRef();
+}
+
+WeakReference::Flag* WeakReference::Flag::NullFlag() {
+ ANNOTATE_SCOPED_MEMORY_LEAK;
+ static Flag* g_null_flag = new Flag(kNullFlagTag);
+ return g_null_flag;
}
void WeakReference::Flag::Invalidate() {
+#if DCHECK_IS_ON()
+ if (this == NullFlag()) {
+ // The Null Flag does not participate in the sequence checks below.
+ // Since its state never changes, it can be accessed from any thread.
+ DCHECK(!is_valid_);
+ return;
+ }
// The flag being invalidated with a single ref implies that there are no
- // weak pointers in existence. Allow deletion on other thread in this case.
+ // weak pointers in existence. Allow deletion on other thread in this
+ // case.
DCHECK(sequence_checker_.CalledOnValidSequence() || HasOneRef())
<< "WeakPtrs must be invalidated on the same sequenced thread.";
- is_valid_ = false;
+#endif
+ is_valid_ = 0;
}
-bool WeakReference::Flag::IsValid() const {
- DCHECK(sequence_checker_.CalledOnValidSequence())
- << "WeakPtrs must be checked on the same sequenced thread.";
- return is_valid_;
-}
+WeakReference::Flag::~Flag() {}
-WeakReference::Flag::~Flag() {
-}
+WeakReference::WeakReference() : flag_(Flag::NullFlag()) {}
-WeakReference::WeakReference() {
-}
+WeakReference::~WeakReference() {}
-WeakReference::WeakReference(const Flag* flag) : flag_(flag) {
-}
+WeakReference::WeakReference(const Flag* flag) : flag_(flag) {}
-WeakReference::~WeakReference() {
+WeakReference::WeakReference(WeakReference&& other)
+ : flag_(std::move(other.flag_)) {
+ other.flag_ = Flag::NullFlag();
}
-WeakReference::WeakReference(WeakReference&& other) = default;
-
WeakReference::WeakReference(const WeakReference& other) = default;
-bool WeakReference::is_valid() const { return flag_.get() && flag_->IsValid(); }
-
-WeakReferenceOwner::WeakReferenceOwner() {
-}
+WeakReferenceOwner::WeakReferenceOwner()
+ : flag_(WeakReference::Flag::NullFlag()) {}
WeakReferenceOwner::~WeakReferenceOwner() {
Invalidate();
@@ -62,9 +83,9 @@ WeakReference WeakReferenceOwner::GetRef() const {
}
void WeakReferenceOwner::Invalidate() {
- if (flag_.get()) {
+ if (flag_ != WeakReference::Flag::NullFlag()) {
flag_->Invalidate();
- flag_ = NULL;
+ flag_ = WeakReference::Flag::NullFlag();
}
}

Powered by Google App Engine
This is Rietveld 408576698