diff --git a/source/blender/blenkernel/BKE_anonymous_attribute_id.hh b/source/blender/blenkernel/BKE_anonymous_attribute_id.hh index 7e4ca4b7afd..d9024ed9c05 100644 --- a/source/blender/blenkernel/BKE_anonymous_attribute_id.hh +++ b/source/blender/blenkernel/BKE_anonymous_attribute_id.hh @@ -4,9 +4,9 @@ #include +#include "BLI_implicit_sharing_ptr.hh" #include "BLI_set.hh" #include "BLI_string_ref.hh" -#include "BLI_user_counter.hh" namespace blender::bke { @@ -32,10 +32,7 @@ namespace blender::bke { * because that is not available in C code. If possible, the #AutoAnonymousAttributeID wrapper * should be used to avoid manual reference counting in C++ code. */ -class AnonymousAttributeID { - private: - mutable std::atomic users_ = 1; - +class AnonymousAttributeID : public ImplicitSharingMixin { protected: std::string name_; @@ -49,22 +46,15 @@ class AnonymousAttributeID { virtual std::string user_name() const; - void user_add() const + private: + void delete_self() override { - users_.fetch_add(1); - } - - void user_remove() const - { - const int new_users = users_.fetch_sub(1) - 1; - if (new_users == 0) { - MEM_delete(this); - } + MEM_delete(this); } }; /** Wrapper for #AnonymousAttributeID that avoids manual reference counting. */ -using AutoAnonymousAttributeID = UserCounter; +using AutoAnonymousAttributeID = ImplicitSharingPtr; /** * A set of anonymous attribute names that is passed around in geometry nodes. diff --git a/source/blender/blenkernel/BKE_geometry_set.hh b/source/blender/blenkernel/BKE_geometry_set.hh index 72f77c889c1..a70fa8cfb4e 100644 --- a/source/blender/blenkernel/BKE_geometry_set.hh +++ b/source/blender/blenkernel/BKE_geometry_set.hh @@ -12,8 +12,6 @@ #include "BLI_function_ref.hh" #include "BLI_map.hh" -#include "BLI_math_vector_types.hh" -#include "BLI_user_counter.hh" #include "BLI_vector_set.hh" #include "BKE_attribute.hh" @@ -40,18 +38,13 @@ class CurvesEditHints; class Instances; } // namespace blender::bke -class GeometryComponent; - /** - * This is the base class for specialized geometry component types. A geometry component handles - * a user count to allow avoiding duplication when it is wrapped with #UserCounter. It also handles - * the attribute API, which generalizes storing and modifying generic information on a geometry. + * This is the base class for specialized geometry component types. A geometry component uses + * implicit sharing to avoid read-only copies. It also integrates with attribute API, which + * generalizes storing and modifying generic information on a geometry. */ -class GeometryComponent { +class GeometryComponent : public blender::ImplicitSharingMixin { private: - /* The reference count has two purposes. When it becomes zero, the component is freed. When it is - * larger than one, the component becomes immutable. */ - mutable std::atomic users_ = 1; GeometryComponentType type_; public: @@ -77,13 +70,12 @@ class GeometryComponent { virtual bool owns_direct_data() const = 0; virtual void ensure_owns_direct_data() = 0; - void user_add() const; - void user_remove() const; - bool is_mutable() const; - GeometryComponentType type() const; virtual bool is_empty() const; + + private: + void delete_self() override; }; template @@ -109,7 +101,7 @@ inline constexpr bool is_geometry_component_v = std::is_base_of_v; + using GeometryComponentPtr = blender::ImplicitSharingPtr; /* Indexed by #GeometryComponentType. */ std::array components_; diff --git a/source/blender/blenkernel/intern/customdata.cc b/source/blender/blenkernel/intern/customdata.cc index a49ff002d91..72636e2176c 100644 --- a/source/blender/blenkernel/intern/customdata.cc +++ b/source/blender/blenkernel/intern/customdata.cc @@ -2273,7 +2273,7 @@ bool CustomData_merge(const CustomData *source, layer->anonymous_id = nullptr; } else { - layer->anonymous_id->user_add(); + layer->anonymous_id->add_user(); } } if (alloctype == CD_ASSIGN) { @@ -2365,7 +2365,7 @@ static void customData_free_layer__internal(CustomDataLayer *layer, const int to const LayerTypeInfo *typeInfo; if (layer->anonymous_id != nullptr) { - layer->anonymous_id->user_remove(); + layer->anonymous_id->remove_user_and_delete_if_last(); layer->anonymous_id = nullptr; } if (!(layer->flag & CD_FLAG_NOFREE) && layer->data) { @@ -2956,7 +2956,7 @@ void *CustomData_add_layer_anonymous(CustomData *data, return nullptr; } - anonymous_id->user_add(); + anonymous_id->add_user(); layer->anonymous_id = anonymous_id; return layer->data; } diff --git a/source/blender/blenkernel/intern/geometry_set.cc b/source/blender/blenkernel/intern/geometry_set.cc index 3f5fadbe636..713034e4f16 100644 --- a/source/blender/blenkernel/intern/geometry_set.cc +++ b/source/blender/blenkernel/intern/geometry_set.cc @@ -84,26 +84,6 @@ std::optional GeometryComponent::attribu return std::nullopt; } -void GeometryComponent::user_add() const -{ - users_.fetch_add(1); -} - -void GeometryComponent::user_remove() const -{ - const int new_users = users_.fetch_sub(1) - 1; - if (new_users == 0) { - delete this; - } -} - -bool GeometryComponent::is_mutable() const -{ - /* If the item is shared, it is read-only. */ - /* The user count can be 0, when this is called from the destructor. */ - return users_ <= 1; -} - GeometryComponentType GeometryComponent::type() const { return type_; @@ -114,6 +94,11 @@ bool GeometryComponent::is_empty() const return false; } +void GeometryComponent::delete_self() +{ + delete this; +} + /** \} */ /* -------------------------------------------------------------------- */ @@ -198,7 +183,7 @@ void GeometrySet::remove_geometry_during_modify() void GeometrySet::add(const GeometryComponent &component) { BLI_assert(!components_[component.type()]); - component.user_add(); + component.add_user(); components_[component.type()] = const_cast(&component); } diff --git a/source/blender/blenlib/BLI_implicit_sharing.hh b/source/blender/blenlib/BLI_implicit_sharing.hh new file mode 100644 index 00000000000..7b47830aadf --- /dev/null +++ b/source/blender/blenlib/BLI_implicit_sharing.hh @@ -0,0 +1,109 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#pragma once + +/** \file + * \ingroup bli + */ + +#include + +#include "BLI_compiler_attrs.h" +#include "BLI_utildefines.h" +#include "BLI_utility_mixins.hh" + +namespace blender { + +/** + * #ImplicitSharingInfo is the core data structure for implicit sharing in Blender. Implicit + * sharing is a technique that avoids copying data when it is not necessary. This results in better + * memory usage and performance. Only read-only data can be shared, because otherwise multiple + * owners might want to change the data in conflicting ways. + * + * To determine whether data is shared, #ImplicitSharingInfo keeps a user count. If the count is 1, + * the data only has a single owner and is therefore mutable. If some code wants to modify data + * that is currently shared, it has to make a copy first. + * This behavior is also called "copy on write". + * + * In addition to containing the reference count, #ImplicitSharingInfo also knows how to destruct + * the referenced data. This is important because the code freeing the data in the end might not + * know how it was allocated (for example, it doesn't know whether an array was allocated using the + * system or guarded allocator). + * + * #ImplicitSharingInfo can be used in two ways: + * - It can be allocated separately from the referenced data. This is used when the shared data is + * e.g. a plain data array. + * - It can be embedded into another struct. For that it's best to use #ImplicitSharingMixin. + */ +class ImplicitSharingInfo : NonCopyable, NonMovable { + private: + mutable std::atomic users_; + + public: + ImplicitSharingInfo(const int initial_users) : users_(initial_users) + { + } + + virtual ~ImplicitSharingInfo() + { + BLI_assert(this->is_mutable()); + } + + /** True if there are other const references to the resource, meaning it cannot be modified. */ + bool is_shared() const + { + return users_.load(std::memory_order_relaxed) >= 2; + } + + /** Whether the resource can be modified without a copy because there is only one owner. */ + bool is_mutable() const + { + return !this->is_shared(); + } + + /** Call when a the data has a new additional owner. */ + void add_user() const + { + users_.fetch_add(1, std::memory_order_relaxed); + } + + /** + * Call when the data is no longer needed. This might just decrement the user count, or it might + * also delete the data if this was the last user. + */ + void remove_user_and_delete_if_last() const + { + const int old_user_count = users_.fetch_sub(1, std::memory_order_acq_rel); + BLI_assert(old_user_count >= 1); + const bool was_last_user = old_user_count == 1; + if (was_last_user) { + const_cast(this)->delete_self_with_data(); + } + } + + private: + /** Has to free the #ImplicitSharingInfo and the referenced data. */ + virtual void delete_self_with_data() = 0; +}; + +/** + * Makes it easy to embed implicit-sharing behavior into a struct. Structs that derive from this + * class can be used with #ImplicitSharingPtr. + */ +class ImplicitSharingMixin : public ImplicitSharingInfo { + public: + ImplicitSharingMixin() : ImplicitSharingInfo(1) + { + } + + private: + void delete_self_with_data() override + { + /* Can't use `delete this` here, because we don't know what allocator was used. */ + this->delete_self(); + } + + virtual void delete_self() = 0; +}; + +} // namespace blender diff --git a/source/blender/blenlib/BLI_user_counter.hh b/source/blender/blenlib/BLI_implicit_sharing_ptr.hh similarity index 53% rename from source/blender/blenlib/BLI_user_counter.hh rename to source/blender/blenlib/BLI_implicit_sharing_ptr.hh index 11cd845ff53..8e726ac97ce 100644 --- a/source/blender/blenlib/BLI_user_counter.hh +++ b/source/blender/blenlib/BLI_implicit_sharing_ptr.hh @@ -6,59 +6,60 @@ * \ingroup bli */ -#include +#include "BLI_implicit_sharing.hh" namespace blender { /** - * A simple automatic reference counter. It is similar to std::shared_ptr, but expects that the - * reference count is inside the object. + * #ImplicitSharingPtr is a smart pointer that manages implicit sharing. It's designed to work with + * types that derive from #ImplicitSharingMixin. It is fairly similar to #std::shared_ptr but + * requires the reference count to be embedded in the data. */ -template class UserCounter { +template class ImplicitSharingPtr { private: T *data_ = nullptr; public: - UserCounter() = default; + ImplicitSharingPtr() = default; - UserCounter(T *data) : data_(data) + ImplicitSharingPtr(T *data) : data_(data) { } - UserCounter(const UserCounter &other) : data_(other.data_) + ImplicitSharingPtr(const ImplicitSharingPtr &other) : data_(other.data_) { - this->user_add(data_); + this->add_user(data_); } - UserCounter(UserCounter &&other) : data_(other.data_) + ImplicitSharingPtr(ImplicitSharingPtr &&other) : data_(other.data_) { other.data_ = nullptr; } - ~UserCounter() + ~ImplicitSharingPtr() { - this->user_remove(data_); + this->remove_user_and_delete_if_last(data_); } - UserCounter &operator=(const UserCounter &other) + ImplicitSharingPtr &operator=(const ImplicitSharingPtr &other) { if (this == &other) { return *this; } - this->user_remove(data_); + this->remove_user_and_delete_if_last(data_); data_ = other.data_; - this->user_add(data_); + this->add_user(data_); return *this; } - UserCounter &operator=(UserCounter &&other) + ImplicitSharingPtr &operator=(ImplicitSharingPtr &&other) { if (this == &other) { return *this; } - this->user_remove(data_); + this->remove_user_and_delete_if_last(data_); data_ = other.data_; other.data_ = nullptr; return *this; @@ -112,7 +113,7 @@ template class UserCounter { void reset() { - this->user_remove(data_); + this->remove_user_and_delete_if_last(data_); data_ = nullptr; } @@ -126,29 +127,23 @@ template class UserCounter { return get_default_hash(data_); } - friend bool operator==(const UserCounter &a, const UserCounter &b) + friend bool operator==(const ImplicitSharingPtr &a, const ImplicitSharingPtr &b) { return a.data_ == b.data_; } - friend std::ostream &operator<<(std::ostream &stream, const UserCounter &value) - { - stream << value.data_; - return stream; - } - private: - static void user_add(T *data) + static void add_user(T *data) { if (data != nullptr) { - data->user_add(); + data->add_user(); } } - static void user_remove(T *data) + static void remove_user_and_delete_if_last(T *data) { if (data != nullptr) { - data->user_remove(); + data->remove_user_and_delete_if_last(); } } }; diff --git a/source/blender/blenlib/CMakeLists.txt b/source/blender/blenlib/CMakeLists.txt index c4c94ed001e..04ba4a672ee 100644 --- a/source/blender/blenlib/CMakeLists.txt +++ b/source/blender/blenlib/CMakeLists.txt @@ -243,6 +243,8 @@ set(SRC BLI_hash_tables.hh BLI_heap.h BLI_heap_simple.h + BLI_implicit_sharing.hh + BLI_implicit_sharing_ptr.hh BLI_index_mask.hh BLI_index_mask_ops.hh BLI_index_range.hh @@ -355,7 +357,6 @@ set(SRC BLI_timecode.h BLI_timeit.hh BLI_timer.h - BLI_user_counter.hh BLI_utildefines.h BLI_utildefines_iter.h BLI_utildefines_stack.h @@ -493,6 +494,7 @@ if(WITH_GTESTS) tests/BLI_hash_mm2a_test.cc tests/BLI_heap_simple_test.cc tests/BLI_heap_test.cc + tests/BLI_implicit_sharing_test.cc tests/BLI_index_mask_test.cc tests/BLI_index_range_test.cc tests/BLI_inplace_priority_queue_test.cc diff --git a/source/blender/blenlib/tests/BLI_implicit_sharing_test.cc b/source/blender/blenlib/tests/BLI_implicit_sharing_test.cc new file mode 100644 index 00000000000..ebb341d7a1b --- /dev/null +++ b/source/blender/blenlib/tests/BLI_implicit_sharing_test.cc @@ -0,0 +1,85 @@ +/* SPDX-License-Identifier: Apache-2.0 */ + +#include "MEM_guardedalloc.h" + +#include "BLI_implicit_sharing_ptr.hh" + +#include "testing/testing.h" + +namespace blender::tests { + +class ImplicitlySharedData : public ImplicitSharingMixin { + public: + ImplicitSharingPtr copy() const + { + return MEM_new(__func__); + } + + void delete_self() override + { + MEM_delete(this); + } +}; + +class SharedDataContainer { + private: + ImplicitSharingPtr data_; + + public: + SharedDataContainer() : data_(MEM_new(__func__)) + { + } + + const ImplicitlySharedData *get_for_read() const + { + return data_.get(); + } + + ImplicitlySharedData *get_for_write() + { + if (!data_) { + return nullptr; + } + if (data_->is_mutable()) { + return data_.get(); + } + data_ = data_->copy(); + return data_.get(); + } +}; + +TEST(implicit_sharing, CopyOnWriteAccess) +{ + /* Create the initial data. */ + SharedDataContainer a; + EXPECT_NE(a.get_for_read(), nullptr); + + /* a and b share the same underlying data now. */ + SharedDataContainer b = a; + EXPECT_EQ(a.get_for_read(), b.get_for_read()); + + /* c now shares the data with a and b. */ + SharedDataContainer c = a; + EXPECT_EQ(b.get_for_read(), c.get_for_read()); + + /* Retrieving write access on b should make a copy because the data is shared. */ + ImplicitlySharedData *data_b1 = b.get_for_write(); + EXPECT_NE(data_b1, nullptr); + EXPECT_EQ(data_b1, b.get_for_read()); + EXPECT_NE(data_b1, a.get_for_read()); + EXPECT_NE(data_b1, c.get_for_read()); + + /* Retrieving the same write access again should *not* make another copy. */ + ImplicitlySharedData *data_b2 = b.get_for_write(); + EXPECT_EQ(data_b1, data_b2); + + /* Moving b should also move the data. b then does not have ownership anymore. Since the data in + * b only had one owner, the data is still mutable now that d is the owner. */ + SharedDataContainer d = std::move(b); + EXPECT_EQ(b.get_for_read(), nullptr); + EXPECT_EQ(b.get_for_write(), nullptr); + EXPECT_EQ(d.get_for_read(), data_b1); + EXPECT_EQ(d.get_for_write(), data_b1); +} + +} // namespace blender::tests diff --git a/source/blender/geometry/intern/mesh_split_edges.cc b/source/blender/geometry/intern/mesh_split_edges.cc index 007ec7c3b63..00cfa5eb756 100644 --- a/source/blender/geometry/intern/mesh_split_edges.cc +++ b/source/blender/geometry/intern/mesh_split_edges.cc @@ -2,7 +2,6 @@ #include "BLI_array_utils.hh" #include "BLI_index_mask.hh" -#include "BLI_user_counter.hh" #include "BKE_attribute.hh" #include "BKE_attribute_math.hh" @@ -69,7 +68,7 @@ static void add_new_edges(Mesh &mesh, /* Store a copy of the IDs locally since we will remove the existing attributes which * can also free the names, since the API does not provide pointer stability. */ Vector named_ids; - Vector> anonymous_ids; + Vector anonymous_ids; for (const bke::AttributeIDRef &id : attributes.all_ids()) { if (attributes.lookup_meta_data(id)->domain != ATTR_DOMAIN_EDGE) { continue; @@ -82,14 +81,14 @@ static void add_new_edges(Mesh &mesh, } else { anonymous_ids.append(&id.anonymous_id()); - id.anonymous_id().user_add(); + id.anonymous_id().add_user(); } } Vector local_edge_ids; for (const StringRef name : named_ids) { local_edge_ids.append(name); } - for (const UserCounter &id : anonymous_ids) { + for (const bke::AutoAnonymousAttributeID &id : anonymous_ids) { local_edge_ids.append(*id); } diff --git a/source/blender/geometry/intern/realize_instances.cc b/source/blender/geometry/intern/realize_instances.cc index d53446cdd5f..e157e05c4c6 100644 --- a/source/blender/geometry/intern/realize_instances.cc +++ b/source/blender/geometry/intern/realize_instances.cc @@ -229,8 +229,8 @@ struct GatherTasks { /* Volumes only have very simple support currently. Only the first found volume is put into the * output. */ - UserCounter first_volume; - UserCounter first_edit_data; + ImplicitSharingPtr first_volume; + ImplicitSharingPtr first_edit_data; }; /** Current offsets while during the gather operation. */ @@ -611,7 +611,7 @@ static void gather_realize_tasks_recursive(GatherTasksInfo &gather_info, case GEO_COMPONENT_TYPE_VOLUME: { const VolumeComponent *volume_component = static_cast(component); if (!gather_info.r_tasks.first_volume) { - volume_component->user_add(); + volume_component->add_user(); gather_info.r_tasks.first_volume = volume_component; } break; @@ -620,7 +620,7 @@ static void gather_realize_tasks_recursive(GatherTasksInfo &gather_info, const GeometryComponentEditData *edit_component = static_cast(component); if (!gather_info.r_tasks.first_edit_data) { - edit_component->user_add(); + edit_component->add_user(); gather_info.r_tasks.first_edit_data = edit_component; } break;