diff options
author | Jacques Lucke <jacques@blender.org> | 2021-09-14 17:08:09 +0300 |
---|---|---|
committer | Jacques Lucke <jacques@blender.org> | 2021-09-14 17:08:09 +0300 |
commit | dee0b56b9216de8f37589b15be2d21cc1b946773 (patch) | |
tree | 986b930459ac12d7230e80afd41476c38d9b5bc3 | |
parent | 426e2663a0891d16a497a33b273a5cee1e09f929 (diff) |
Cleanup: simplify resource scope methods
Previously, a debug name had to be passed to all methods
that added a resource to the `ResourceScope`. The idea was
that this would make it easier to find certain bugs. In reality
I never found this to be useful, and it was mostly annoying.
The thing is, something that is in a resource scope never leaks
(unless the resource scope is not destructed of course).
Removing the name parameter makes the structure easier to use.
12 files changed, 67 insertions, 98 deletions
diff --git a/source/blender/blenkernel/intern/attribute_access.cc b/source/blender/blenkernel/intern/attribute_access.cc index 2a5bb99a18b..bdf1891a55a 100644 --- a/source/blender/blenkernel/intern/attribute_access.cc +++ b/source/blender/blenkernel/intern/attribute_access.cc @@ -1315,7 +1315,7 @@ const GVArray *AttributeFieldInput::get_varray_for_context(const fn::FieldContex const AttributeDomain domain = geometry_context->domain(); const CustomDataType data_type = cpp_type_to_custom_data_type(*type_); GVArrayPtr attribute = component.attribute_try_get_for_read(name_, domain, data_type); - return scope.add(std::move(attribute), __func__); + return scope.add(std::move(attribute)); } return nullptr; } @@ -1350,7 +1350,7 @@ const GVArray *AnonymousAttributeFieldInput::get_varray_for_context( const CustomDataType data_type = cpp_type_to_custom_data_type(*type_); GVArrayPtr attribute = component.attribute_try_get_for_read( anonymous_id_.get(), domain, data_type); - return scope.add(std::move(attribute), __func__); + return scope.add(std::move(attribute)); } return nullptr; } diff --git a/source/blender/blenlib/BLI_resource_scope.hh b/source/blender/blenlib/BLI_resource_scope.hh index b7720b52ecc..edffb148477 100644 --- a/source/blender/blenlib/BLI_resource_scope.hh +++ b/source/blender/blenlib/BLI_resource_scope.hh @@ -50,11 +50,10 @@ class ResourceScope : NonCopyable, NonMovable { struct ResourceData { void *data; void (*free)(void *data); - const char *debug_name; }; - LinearAllocator<> m_allocator; - Vector<ResourceData> m_resources; + LinearAllocator<> allocator_; + Vector<ResourceData> resources_; public: ResourceScope() = default; @@ -62,8 +61,8 @@ class ResourceScope : NonCopyable, NonMovable { ~ResourceScope() { /* Free in reversed order. */ - for (int64_t i = m_resources.size(); i--;) { - ResourceData &data = m_resources[i]; + for (int64_t i = resources_.size(); i--;) { + ResourceData &data = resources_[i]; data.free(data.data); } } @@ -72,20 +71,17 @@ class ResourceScope : NonCopyable, NonMovable { * Pass ownership of the resource to the ResourceScope. It will be destructed and freed when * the collector is destructed. */ - template<typename T> T *add(std::unique_ptr<T> resource, const char *name) + template<typename T> T *add(std::unique_ptr<T> resource) { BLI_assert(resource.get() != nullptr); T *ptr = resource.release(); if (ptr == nullptr) { return nullptr; } - this->add( - ptr, - [](void *data) { - T *typed_data = reinterpret_cast<T *>(data); - delete typed_data; - }, - name); + this->add(ptr, [](void *data) { + T *typed_data = reinterpret_cast<T *>(data); + delete typed_data; + }); return ptr; } @@ -93,7 +89,7 @@ class ResourceScope : NonCopyable, NonMovable { * Pass ownership of the resource to the ResourceScope. It will be destructed when the * collector is destructed. */ - template<typename T> T *add(destruct_ptr<T> resource, const char *name) + template<typename T> T *add(destruct_ptr<T> resource) { T *ptr = resource.release(); if (ptr == nullptr) { @@ -104,13 +100,10 @@ class ResourceScope : NonCopyable, NonMovable { return ptr; } - this->add( - ptr, - [](void *data) { - T *typed_data = reinterpret_cast<T *>(data); - typed_data->~T(); - }, - name); + this->add(ptr, [](void *data) { + T *typed_data = reinterpret_cast<T *>(data); + typed_data->~T(); + }); return ptr; } @@ -118,33 +111,31 @@ class ResourceScope : NonCopyable, NonMovable { * Pass ownership of some resource to the ResourceScope. The given free function will be * called when the collector is destructed. */ - void add(void *userdata, void (*free)(void *), const char *name) + void add(void *userdata, void (*free)(void *)) { ResourceData data; - data.debug_name = name; data.data = userdata; data.free = free; - m_resources.append(data); + resources_.append(data); } /** * Construct an object with the same value in the ResourceScope and return a reference to the * new value. */ - template<typename T> T &add_value(T &&value, const char *name) + template<typename T> T &add_value(T &&value) { - return this->construct<T>(name, std::forward<T>(value)); + return this->construct<T>(std::forward<T>(value)); } /** * The passed in function will be called when the scope is destructed. */ - template<typename Func> void add_destruct_call(Func func, const char *name) + template<typename Func> void add_destruct_call(Func func) { - void *buffer = m_allocator.allocate(sizeof(Func), alignof(Func)); + void *buffer = allocator_.allocate(sizeof(Func), alignof(Func)); new (buffer) Func(std::move(func)); - this->add( - buffer, [](void *data) { (*(Func *)data)(); }, name); + this->add(buffer, [](void *data) { (*(Func *)data)(); }); } /** @@ -153,37 +144,19 @@ class ResourceScope : NonCopyable, NonMovable { */ LinearAllocator<> &linear_allocator() { - return m_allocator; + return allocator_; } /** * Utility method to construct an instance of type T that will be owned by the ResourceScope. */ - template<typename T, typename... Args> T &construct(const char *name, Args &&...args) + template<typename T, typename... Args> T &construct(Args &&...args) { - destruct_ptr<T> value_ptr = m_allocator.construct<T>(std::forward<Args>(args)...); + destruct_ptr<T> value_ptr = allocator_.construct<T>(std::forward<Args>(args)...); T &value_ref = *value_ptr; - this->add(std::move(value_ptr), name); + this->add(std::move(value_ptr)); return value_ref; } - - /** - * Print the names of all the resources that are owned by this ResourceScope. This can be - * useful for debugging. - */ - void print(StringRef name) const - { - if (m_resources.size() == 0) { - std::cout << "\"" << name << "\" has no resources.\n"; - return; - } - else { - std::cout << "Resources for \"" << name << "\":\n"; - for (const ResourceData &data : m_resources) { - std::cout << " " << data.data << ": " << data.debug_name << '\n'; - } - } - } }; } // namespace blender diff --git a/source/blender/editors/space_spreadsheet/space_spreadsheet.cc b/source/blender/editors/space_spreadsheet/space_spreadsheet.cc index d503297f540..a82648aeee0 100644 --- a/source/blender/editors/space_spreadsheet/space_spreadsheet.cc +++ b/source/blender/editors/space_spreadsheet/space_spreadsheet.cc @@ -370,7 +370,7 @@ static void spreadsheet_main_region_draw(const bContext *C, ARegion *region) std::unique_ptr<ColumnValues> values_ptr = data_source->get_column_values(*column->id); /* Should have been removed before if it does not exist anymore. */ BLI_assert(values_ptr); - const ColumnValues *values = scope.add(std::move(values_ptr), __func__); + const ColumnValues *values = scope.add(std::move(values_ptr)); const int width = get_column_width_in_pixels(*values); spreadsheet_layout.columns.append({values, width}); diff --git a/source/blender/editors/space_spreadsheet/spreadsheet_data_source_geometry.cc b/source/blender/editors/space_spreadsheet/spreadsheet_data_source_geometry.cc index f5fef5e4486..bd2d89e4f27 100644 --- a/source/blender/editors/space_spreadsheet/spreadsheet_data_source_geometry.cc +++ b/source/blender/editors/space_spreadsheet/spreadsheet_data_source_geometry.cc @@ -70,7 +70,7 @@ std::unique_ptr<ColumnValues> GeometryDataSource::get_column_values( if (!attribute) { return {}; } - const fn::GVArray *varray = scope_.add(std::move(attribute.varray), __func__); + const fn::GVArray *varray = scope_.add(std::move(attribute.varray)); if (attribute.domain != domain_) { return {}; } diff --git a/source/blender/editors/space_spreadsheet/spreadsheet_row_filter.cc b/source/blender/editors/space_spreadsheet/spreadsheet_row_filter.cc index ae336edfead..1e46fef8d71 100644 --- a/source/blender/editors/space_spreadsheet/spreadsheet_row_filter.cc +++ b/source/blender/editors/space_spreadsheet/spreadsheet_row_filter.cc @@ -328,7 +328,7 @@ Span<int64_t> spreadsheet_filter_rows(const SpaceSpreadsheet &sspreadsheet, geometry_data_source->apply_selection_filter(rows_included); } - Vector<int64_t> &indices = scope.construct<Vector<int64_t>>(__func__); + Vector<int64_t> &indices = scope.construct<Vector<int64_t>>(); index_vector_from_bools(rows_included, indices); return indices; diff --git a/source/blender/functions/FN_field.hh b/source/blender/functions/FN_field.hh index d6259bce435..d4375b625ce 100644 --- a/source/blender/functions/FN_field.hh +++ b/source/blender/functions/FN_field.hh @@ -381,7 +381,7 @@ class FieldEvaluator : NonMovable, NonCopyable { /** Same as #add_with_destination but typed. */ template<typename T> int add_with_destination(Field<T> field, VMutableArray<T> &dst) { - GVMutableArray &varray = scope_.construct<GVMutableArray_For_VMutableArray<T>>(__func__, dst); + GVMutableArray &varray = scope_.construct<GVMutableArray_For_VMutableArray<T>>(dst); return this->add_with_destination(GField(std::move(field)), varray); } @@ -401,7 +401,7 @@ class FieldEvaluator : NonMovable, NonCopyable { */ template<typename T> int add_with_destination(Field<T> field, MutableSpan<T> dst) { - GVMutableArray &varray = scope_.construct<GVMutableArray_For_MutableSpan<T>>(__func__, dst); + GVMutableArray &varray = scope_.construct<GVMutableArray_For_MutableSpan<T>>(dst); return this->add_with_destination(std::move(field), varray); } @@ -417,10 +417,10 @@ class FieldEvaluator : NonMovable, NonCopyable { { const int field_index = fields_to_evaluate_.append_and_get_index(std::move(field)); dst_varrays_.append(nullptr); - output_pointer_infos_.append(OutputPointerInfo{ - varray_ptr, [](void *dst, const GVArray &varray, ResourceScope &scope) { - *(const VArray<T> **)dst = &*scope.construct<GVArray_Typed<T>>(__func__, varray); - }}); + output_pointer_infos_.append( + OutputPointerInfo{varray_ptr, [](void *dst, const GVArray &varray, ResourceScope &scope) { + *(const VArray<T> **)dst = &*scope.construct<GVArray_Typed<T>>(varray); + }}); return field_index; } @@ -443,7 +443,7 @@ class FieldEvaluator : NonMovable, NonCopyable { template<typename T> const VArray<T> &get_evaluated(const int field_index) { const GVArray &varray = this->get_evaluated(field_index); - GVArray_Typed<T> &typed_varray = scope_.construct<GVArray_Typed<T>>(__func__, varray); + GVArray_Typed<T> &typed_varray = scope_.construct<GVArray_Typed<T>>(varray); return *typed_varray; } diff --git a/source/blender/functions/FN_multi_function_params.hh b/source/blender/functions/FN_multi_function_params.hh index cae105e7c19..fe4d2b90d80 100644 --- a/source/blender/functions/FN_multi_function_params.hh +++ b/source/blender/functions/FN_multi_function_params.hh @@ -62,25 +62,24 @@ class MFParamsBuilder { template<typename T> void add_readonly_single_input_value(T value, StringRef expected_name = "") { - T *value_ptr = &scope_.add_value<T>(std::move(value), __func__); + T *value_ptr = &scope_.add_value<T>(std::move(value)); this->add_readonly_single_input(value_ptr, expected_name); } template<typename T> void add_readonly_single_input(const T *value, StringRef expected_name = "") { - this->add_readonly_single_input(scope_.construct<GVArray_For_SingleValueRef>( - __func__, CPPType::get<T>(), min_array_size_, value), - expected_name); + this->add_readonly_single_input( + scope_.construct<GVArray_For_SingleValueRef>(CPPType::get<T>(), min_array_size_, value), + expected_name); } void add_readonly_single_input(const GSpan span, StringRef expected_name = "") { - this->add_readonly_single_input(scope_.construct<GVArray_For_GSpan>(__func__, span), - expected_name); + this->add_readonly_single_input(scope_.construct<GVArray_For_GSpan>(span), expected_name); } void add_readonly_single_input(GPointer value, StringRef expected_name = "") { - this->add_readonly_single_input(scope_.construct<GVArray_For_SingleValueRef>( - __func__, *value.type(), min_array_size_, value.get()), - expected_name); + this->add_readonly_single_input( + scope_.construct<GVArray_For_SingleValueRef>(*value.type(), min_array_size_, value.get()), + expected_name); } void add_readonly_single_input(const GVArray &ref, StringRef expected_name = "") { @@ -91,13 +90,13 @@ class MFParamsBuilder { void add_readonly_vector_input(const GVectorArray &vector_array, StringRef expected_name = "") { - this->add_readonly_vector_input( - scope_.construct<GVVectorArray_For_GVectorArray>(__func__, vector_array), expected_name); + this->add_readonly_vector_input(scope_.construct<GVVectorArray_For_GVectorArray>(vector_array), + expected_name); } void add_readonly_vector_input(const GSpan single_vector, StringRef expected_name = "") { this->add_readonly_vector_input( - scope_.construct<GVVectorArray_For_SingleGSpan>(__func__, single_vector, min_array_size_), + scope_.construct<GVVectorArray_For_SingleGSpan>(single_vector, min_array_size_), expected_name); } void add_readonly_vector_input(const GVVectorArray &ref, StringRef expected_name = "") @@ -225,7 +224,7 @@ class MFParams { template<typename T> const VArray<T> &readonly_single_input(int param_index, StringRef name = "") { const GVArray &array = this->readonly_single_input(param_index, name); - return builder_->scope_.construct<GVArray_Typed<T>>(__func__, array); + return builder_->scope_.construct<GVArray_Typed<T>>(array); } const GVArray &readonly_single_input(int param_index, StringRef name = "") { @@ -266,8 +265,7 @@ class MFParams { if (!type.is_trivially_destructible()) { /* Make sure the temporary elements will be destructed in the end. */ builder_->scope_.add_destruct_call( - [&type, buffer, mask = builder_->mask_]() { type.destruct_indices(buffer, mask); }, - __func__); + [&type, buffer, mask = builder_->mask_]() { type.destruct_indices(buffer, mask); }); } span = GMutableSpan{type, buffer, builder_->min_array_size_}; } @@ -278,7 +276,7 @@ class MFParams { const VVectorArray<T> &readonly_vector_input(int param_index, StringRef name = "") { const GVVectorArray &vector_array = this->readonly_vector_input(param_index, name); - return builder_->scope_.construct<VVectorArray_For_GVVectorArray<T>>(__func__, vector_array); + return builder_->scope_.construct<VVectorArray_For_GVVectorArray<T>>(vector_array); } const GVVectorArray &readonly_vector_input(int param_index, StringRef name = "") { diff --git a/source/blender/functions/intern/field.cc b/source/blender/functions/intern/field.cc index 574a9e6284f..43f28efd002 100644 --- a/source/blender/functions/intern/field.cc +++ b/source/blender/functions/intern/field.cc @@ -92,7 +92,7 @@ static Vector<const GVArray *> get_field_context_inputs( if (varray == nullptr) { const CPPType &type = field_input.cpp_type(); varray = &scope.construct<GVArray_For_SingleValueRef>( - __func__, type, mask.min_array_size(), type.default_value()); + type, mask.min_array_size(), type.default_value()); } field_context_inputs.append(varray); } @@ -237,8 +237,8 @@ static void build_multi_function_procedure_for_fields(MFProcedure &procedure, if (!already_output_variables.add(variable)) { /* One variable can be output at most once. To output the same value twice, we have to make * a copy first. */ - const MultiFunction ©_fn = scope.construct<CustomMF_GenericCopy>( - __func__, "copy", variable->data_type()); + const MultiFunction ©_fn = scope.construct<CustomMF_GenericCopy>("copy", + variable->data_type()); variable = builder.add_call<1>(copy_fn, {variable})[0]; } builder.add_output_parameter(*variable); @@ -381,14 +381,13 @@ Vector<const GVArray *> evaluate_fields(ResourceScope &scope, buffer = scope.linear_allocator().allocate(type.size() * array_size, type.alignment()); /* Make sure that elements in the buffer will be destructed. */ - PartiallyInitializedArray &destruct_helper = scope.construct<PartiallyInitializedArray>( - __func__); + PartiallyInitializedArray &destruct_helper = scope.construct<PartiallyInitializedArray>(); destruct_helper.buffer = buffer; destruct_helper.mask = mask; destruct_helper.type = &type; r_varrays[out_index] = &scope.construct<GVArray_For_GSpan>( - __func__, GSpan{type, buffer, array_size}); + GSpan{type, buffer, array_size}); } else { /* Write the result into the existing span. */ @@ -427,8 +426,7 @@ Vector<const GVArray *> evaluate_fields(ResourceScope &scope, void *buffer = scope.linear_allocator().allocate(type.size(), type.alignment()); /* Use this to make sure that the value is destructed in the end. */ - PartiallyInitializedArray &destruct_helper = scope.construct<PartiallyInitializedArray>( - __func__); + PartiallyInitializedArray &destruct_helper = scope.construct<PartiallyInitializedArray>(); destruct_helper.buffer = buffer; destruct_helper.mask = IndexRange(1); destruct_helper.type = &type; @@ -439,7 +437,7 @@ Vector<const GVArray *> evaluate_fields(ResourceScope &scope, /* Create virtual array that can be used after the procedure has been executed below. */ const int out_index = constant_field_indices[i]; r_varrays[out_index] = &scope.construct<GVArray_For_SingleValueRef>( - __func__, type, array_size, buffer); + type, array_size, buffer); } procedure_executor.call(IndexRange(1), mf_params, mf_context); @@ -608,7 +606,7 @@ int FieldEvaluator::add_with_destination(GField field, GVMutableArray &dst) int FieldEvaluator::add_with_destination(GField field, GMutableSpan dst) { - GVMutableArray &varray = scope_.construct<GVMutableArray_For_GMutableSpan>(__func__, dst); + GVMutableArray &varray = scope_.construct<GVMutableArray_For_GMutableSpan>(dst); return this->add_with_destination(std::move(field), varray); } @@ -661,7 +659,7 @@ IndexMask FieldEvaluator::get_evaluated_as_mask(const int field_index) return IndexRange(0); } - return scope_.add_value(indices_from_selection(*typed_varray), __func__).as_span(); + return scope_.add_value(indices_from_selection(*typed_varray)).as_span(); } } // namespace blender::fn diff --git a/source/blender/functions/tests/FN_field_test.cc b/source/blender/functions/tests/FN_field_test.cc index 1c2d5c8eaad..041cdbd0f00 100644 --- a/source/blender/functions/tests/FN_field_test.cc +++ b/source/blender/functions/tests/FN_field_test.cc @@ -41,7 +41,7 @@ class IndexFieldInput final : public FieldInput { auto index_func = [](int i) { return i; }; return &scope.construct< GVArray_For_EmbeddedVArray<int, VArray_For_Func<int, decltype(index_func)>>>( - __func__, mask.min_array_size(), mask.min_array_size(), index_func); + mask.min_array_size(), mask.min_array_size(), index_func); } }; diff --git a/source/blender/nodes/NOD_multi_function.hh b/source/blender/nodes/NOD_multi_function.hh index 2f4b104fb4c..58816544dc1 100644 --- a/source/blender/nodes/NOD_multi_function.hh +++ b/source/blender/nodes/NOD_multi_function.hh @@ -114,7 +114,7 @@ inline void NodeMultiFunctionBuilder::set_matching_fn(const MultiFunction &fn) template<typename T, typename... Args> inline void NodeMultiFunctionBuilder::construct_and_set_matching_fn(Args &&...args) { - const T &fn = resource_scope_.construct<T>(__func__, std::forward<Args>(args)...); + const T &fn = resource_scope_.construct<T>(std::forward<Args>(args)...); this->set_matching_fn(&fn); } diff --git a/source/blender/nodes/geometry/nodes/node_geo_input_index.cc b/source/blender/nodes/geometry/nodes/node_geo_input_index.cc index e2287abe56c..c52ff3d448e 100644 --- a/source/blender/nodes/geometry/nodes/node_geo_input_index.cc +++ b/source/blender/nodes/geometry/nodes/node_geo_input_index.cc @@ -37,7 +37,7 @@ class IndexFieldInput final : public fn::FieldInput { auto index_func = [](int i) { return i; }; return &scope.construct< fn::GVArray_For_EmbeddedVArray<int, VArray_For_Func<int, decltype(index_func)>>>( - __func__, mask.min_array_size(), mask.min_array_size(), index_func); + mask.min_array_size(), mask.min_array_size(), index_func); } }; diff --git a/source/blender/nodes/geometry/nodes/node_geo_input_normal.cc b/source/blender/nodes/geometry/nodes/node_geo_input_normal.cc index b8f126ef1db..07818f2a3ad 100644 --- a/source/blender/nodes/geometry/nodes/node_geo_input_normal.cc +++ b/source/blender/nodes/geometry/nodes/node_geo_input_normal.cc @@ -104,10 +104,10 @@ static const GVArray *construct_mesh_normals_gvarray(const MeshComponent &mesh_c switch (domain) { case ATTR_DOMAIN_FACE: { - return scope.add_value(mesh_face_normals(mesh, verts, polys, loops, mask), __func__).get(); + return scope.add_value(mesh_face_normals(mesh, verts, polys, loops, mask)).get(); } case ATTR_DOMAIN_POINT: { - return scope.add_value(mesh_vertex_normals(mesh, verts, polys, loops, mask), __func__).get(); + return scope.add_value(mesh_vertex_normals(mesh, verts, polys, loops, mask)).get(); } case ATTR_DOMAIN_EDGE: { /* In this case, start with vertex normals and convert to the edge domain, since the @@ -128,7 +128,7 @@ static const GVArray *construct_mesh_normals_gvarray(const MeshComponent &mesh_c } return &scope.construct<fn::GVArray_For_ArrayContainer<Array<float3>>>( - __func__, std::move(edge_normals)); + std::move(edge_normals)); } case ATTR_DOMAIN_CORNER: { /* The normals on corners are just the mesh's face normals, so start with the face normal @@ -140,7 +140,7 @@ static const GVArray *construct_mesh_normals_gvarray(const MeshComponent &mesh_c * will still be normalized, since the face normal is just copied to every corner. */ GVArrayPtr loop_normals = mesh_component.attribute_try_adapt_domain( std::move(face_normals), ATTR_DOMAIN_FACE, ATTR_DOMAIN_CORNER); - return scope.add_value(std::move(loop_normals), __func__).get(); + return scope.add_value(std::move(loop_normals)).get(); } default: return nullptr; |