[Bf-blender-cvs] [dee0b56b921] master: Cleanup: simplify resource scope methods
Jacques Lucke
noreply at git.blender.org
Tue Sep 14 16:09:08 CEST 2021
Commit: dee0b56b9216de8f37589b15be2d21cc1b946773
Author: Jacques Lucke
Date: Tue Sep 14 16:08:09 2021 +0200
Branches: master
https://developer.blender.org/rBdee0b56b9216de8f37589b15be2d21cc1b946773
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.
===================================================================
M source/blender/blenkernel/intern/attribute_access.cc
M source/blender/blenlib/BLI_resource_scope.hh
M source/blender/editors/space_spreadsheet/space_spreadsheet.cc
M source/blender/editors/space_spreadsheet/spreadsheet_data_source_geometry.cc
M source/blender/editors/space_spreadsheet/spreadsheet_row_filter.cc
M source/blender/functions/FN_field.hh
M source/blender/functions/FN_multi_function_params.hh
M source/blender/functions/intern/field.cc
M source/blender/functions/tests/FN_field_test.cc
M source/blender/nodes/NOD_multi_function.hh
M source/blender/nodes/geometry/nodes/node_geo_input_index.cc
M source/blender/nodes/geometry/nodes/node_geo_input_normal.cc
===================================================================
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 &s
@@ Diff output truncated at 10240 characters. @@
More information about the Bf-blender-cvs
mailing list