Move and improve RefCounted
- Move RefCounted to common (from dawn_native) so that we can use
it from additional places.
- Use EXPECT_ macros instead of ASSERT_ in RefCounted tests for
improved logging on failures.
- Add a missing test for Ref::Detach.
- Plug memory leak in RaceOnReferenceRelease
Change-Id: Iaa7b11b5a6fa146e3c322143279a21a4ac027547
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/19903
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Rafael Cintron <rafael.cintron@microsoft.com>
diff --git a/src/dawn_native/BUILD.gn b/src/dawn_native/BUILD.gn
index 37db856..908128c 100644
--- a/src/dawn_native/BUILD.gn
+++ b/src/dawn_native/BUILD.gn
@@ -230,8 +230,6 @@
"ProgrammablePassEncoder.h",
"Queue.cpp",
"Queue.h",
- "RefCounted.cpp",
- "RefCounted.h",
"RenderBundle.cpp",
"RenderBundle.h",
"RenderBundleEncoder.cpp",
diff --git a/src/dawn_native/CMakeLists.txt b/src/dawn_native/CMakeLists.txt
index 72053f3..638b41f 100644
--- a/src/dawn_native/CMakeLists.txt
+++ b/src/dawn_native/CMakeLists.txt
@@ -102,8 +102,6 @@
"ProgrammablePassEncoder.h"
"Queue.cpp"
"Queue.h"
- "RefCounted.cpp"
- "RefCounted.h"
"RenderBundle.cpp"
"RenderBundle.h"
"RenderBundleEncoder.cpp"
diff --git a/src/dawn_native/ErrorScope.h b/src/dawn_native/ErrorScope.h
index f3a0187..eea066d 100644
--- a/src/dawn_native/ErrorScope.h
+++ b/src/dawn_native/ErrorScope.h
@@ -17,7 +17,7 @@
#include "dawn_native/dawn_platform.h"
-#include "dawn_native/RefCounted.h"
+#include "common/RefCounted.h"
#include <string>
diff --git a/src/dawn_native/ErrorScopeTracker.h b/src/dawn_native/ErrorScopeTracker.h
index 7337eb6..2ca6511 100644
--- a/src/dawn_native/ErrorScopeTracker.h
+++ b/src/dawn_native/ErrorScopeTracker.h
@@ -15,8 +15,8 @@
#ifndef DAWNNATIVE_ERRORSCOPETRACKER_H_
#define DAWNNATIVE_ERRORSCOPETRACKER_H_
+#include "common/RefCounted.h"
#include "common/SerialQueue.h"
-#include "dawn_native/RefCounted.h"
namespace dawn_native {
diff --git a/src/dawn_native/FenceSignalTracker.h b/src/dawn_native/FenceSignalTracker.h
index 53333e9..10026a4 100644
--- a/src/dawn_native/FenceSignalTracker.h
+++ b/src/dawn_native/FenceSignalTracker.h
@@ -15,8 +15,8 @@
#ifndef DAWNNATIVE_FENCESIGNALTRACKER_H_
#define DAWNNATIVE_FENCESIGNALTRACKER_H_
+#include "common/RefCounted.h"
#include "common/SerialQueue.h"
-#include "dawn_native/RefCounted.h"
namespace dawn_native {
diff --git a/src/dawn_native/Forward.h b/src/dawn_native/Forward.h
index 05538e8..be7c98f 100644
--- a/src/dawn_native/Forward.h
+++ b/src/dawn_native/Forward.h
@@ -17,6 +17,9 @@
#include <cstdint>
+template <typename T>
+class Ref;
+
namespace dawn_native {
class AdapterBase;
@@ -49,9 +52,6 @@
class DeviceBase;
template <typename T>
- class Ref;
-
- template <typename T>
class PerStage;
struct Format;
diff --git a/src/dawn_native/Instance.h b/src/dawn_native/Instance.h
index e07d04b..f297c03 100644
--- a/src/dawn_native/Instance.h
+++ b/src/dawn_native/Instance.h
@@ -15,10 +15,10 @@
#ifndef DAWNNATIVE_INSTANCE_H_
#define DAWNNATIVE_INSTANCE_H_
+#include "common/RefCounted.h"
#include "dawn_native/Adapter.h"
#include "dawn_native/BackendConnection.h"
#include "dawn_native/Extensions.h"
-#include "dawn_native/RefCounted.h"
#include "dawn_native/Toggles.h"
#include "dawn_native/dawn_platform.h"
diff --git a/src/dawn_native/ObjectBase.h b/src/dawn_native/ObjectBase.h
index cb627a0..544ce1a 100644
--- a/src/dawn_native/ObjectBase.h
+++ b/src/dawn_native/ObjectBase.h
@@ -15,7 +15,7 @@
#ifndef DAWNNATIVE_OBJECTBASE_H_
#define DAWNNATIVE_OBJECTBASE_H_
-#include "dawn_native/RefCounted.h"
+#include "common/RefCounted.h"
namespace dawn_native {
diff --git a/src/dawn_native/RefCounted.cpp b/src/dawn_native/RefCounted.cpp
deleted file mode 100644
index ceb9c75..0000000
--- a/src/dawn_native/RefCounted.cpp
+++ /dev/null
@@ -1,83 +0,0 @@
-// Copyright 2017 The Dawn Authors
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-#include "dawn_native/RefCounted.h"
-
-#include "common/Assert.h"
-
-#include <cstddef>
-
-namespace dawn_native {
-
- static constexpr size_t kPayloadBits = 1;
- static constexpr uint64_t kPayloadMask = (uint64_t(1) << kPayloadBits) - 1;
- static constexpr uint64_t kRefCountIncrement = (uint64_t(1) << kPayloadBits);
-
- RefCounted::RefCounted(uint64_t payload) : mRefCount(kRefCountIncrement + payload) {
- ASSERT((payload & kPayloadMask) == payload);
- }
-
- uint64_t RefCounted::GetRefCountForTesting() const {
- return mRefCount >> kPayloadBits;
- }
-
- uint64_t RefCounted::GetRefCountPayload() const {
- // We only care about the payload bits of the refcount. These never change after
- // initialization so we can use the relaxed memory order. The order doesn't guarantee
- // anything except the atomicity of the load, which is enough since any past values of the
- // atomic will have the correct payload bits.
- return kPayloadMask & mRefCount.load(std::memory_order_relaxed);
- }
-
- void RefCounted::Reference() {
- ASSERT((mRefCount & ~kPayloadMask) != 0);
-
- // The relaxed ordering guarantees only the atomicity of the update, which is enough here
- // because the reference we are copying from still exists and makes sure other threads
- // don't delete `this`.
- // See the explanation in the Boost documentation:
- // https://www.boost.org/doc/libs/1_55_0/doc/html/atomic/usage_examples.html
- mRefCount.fetch_add(kRefCountIncrement, std::memory_order_relaxed);
- }
-
- void RefCounted::Release() {
- ASSERT((mRefCount & ~kPayloadMask) != 0);
-
- // The release fence here is to make sure all accesses to the object on a thread A
- // happen-before the object is deleted on a thread B. The release memory order ensures that
- // all accesses on thread A happen-before the refcount is decreased and the atomic variable
- // makes sure the refcount decrease in A happens-before the refcount decrease in B. Finally
- // the acquire fence in the destruction case makes sure the refcount decrease in B
- // happens-before the `delete this`.
- //
- // See the explanation in the Boost documentation:
- // https://www.boost.org/doc/libs/1_55_0/doc/html/atomic/usage_examples.html
- uint64_t previousRefCount =
- mRefCount.fetch_sub(kRefCountIncrement, std::memory_order_release);
-
- // Check that the previous reference count was strictly less than 2, ignoring payload bits.
- if (previousRefCount < 2 * kRefCountIncrement) {
- // Note that on ARM64 this will generate a `dmb ish` instruction which is a global
- // memory barrier, when an acquire load on mRefCount (using the `ldar` instruction)
- // should be enough and could end up being faster.
- std::atomic_thread_fence(std::memory_order_acquire);
- DeleteThis();
- }
- }
-
- void RefCounted::DeleteThis() {
- delete this;
- }
-
-} // namespace dawn_native
diff --git a/src/dawn_native/RefCounted.h b/src/dawn_native/RefCounted.h
deleted file mode 100644
index 3b3a55e..0000000
--- a/src/dawn_native/RefCounted.h
+++ /dev/null
@@ -1,143 +0,0 @@
-// Copyright 2017 The Dawn Authors
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-#ifndef DAWNNATIVE_REFCOUNTED_H_
-#define DAWNNATIVE_REFCOUNTED_H_
-
-#include <atomic>
-#include <cstdint>
-
-namespace dawn_native {
-
- class RefCounted {
- public:
- RefCounted(uint64_t payload = 0);
-
- uint64_t GetRefCountForTesting() const;
- uint64_t GetRefCountPayload() const;
-
- // Dawn API
- void Reference();
- void Release();
-
- protected:
- virtual ~RefCounted() = default;
- // A Derived class may override this if they require a custom deleter.
- virtual void DeleteThis();
-
- private:
- std::atomic_uint64_t mRefCount;
- };
-
- template <typename T>
- class Ref {
- public:
- Ref() {
- }
-
- Ref(T* p) : mPointee(p) {
- Reference();
- }
-
- Ref(const Ref<T>& other) : mPointee(other.mPointee) {
- Reference();
- }
- Ref<T>& operator=(const Ref<T>& other) {
- if (&other == this)
- return *this;
-
- other.Reference();
- Release();
- mPointee = other.mPointee;
-
- return *this;
- }
-
- Ref(Ref<T>&& other) {
- mPointee = other.mPointee;
- other.mPointee = nullptr;
- }
- Ref<T>& operator=(Ref<T>&& other) {
- if (&other == this)
- return *this;
-
- Release();
- mPointee = other.mPointee;
- other.mPointee = nullptr;
-
- return *this;
- }
-
- ~Ref() {
- Release();
- mPointee = nullptr;
- }
-
- operator bool() {
- return mPointee != nullptr;
- }
-
- const T& operator*() const {
- return *mPointee;
- }
- T& operator*() {
- return *mPointee;
- }
-
- const T* operator->() const {
- return mPointee;
- }
- T* operator->() {
- return mPointee;
- }
-
- const T* Get() const {
- return mPointee;
- }
- T* Get() {
- return mPointee;
- }
-
- T* Detach() {
- T* pointee = mPointee;
- mPointee = nullptr;
- return pointee;
- }
-
- private:
- void Reference() const {
- if (mPointee != nullptr) {
- mPointee->Reference();
- }
- }
- void Release() const {
- if (mPointee != nullptr) {
- mPointee->Release();
- }
- }
-
- // static_assert(std::is_base_of<RefCounted, T>::value, "");
- T* mPointee = nullptr;
- };
-
- template <typename T>
- Ref<T> AcquireRef(T* pointee) {
- Ref<T> ref(pointee);
- ref->Release();
- return ref;
- }
-
-} // namespace dawn_native
-
-#endif // DAWNNATIVE_REFCOUNTED_H_
diff --git a/src/dawn_native/Surface.h b/src/dawn_native/Surface.h
index 96748b7..048298b 100644
--- a/src/dawn_native/Surface.h
+++ b/src/dawn_native/Surface.h
@@ -15,9 +15,9 @@
#ifndef DAWNNATIVE_SURFACE_H_
#define DAWNNATIVE_SURFACE_H_
+#include "common/RefCounted.h"
#include "dawn_native/Error.h"
#include "dawn_native/Forward.h"
-#include "dawn_native/RefCounted.h"
#include "dawn_native/dawn_platform.h"
