WIP: Core: use generic copy-on-write system to avoid redundant copies #104478

Closed
Jacques Lucke wants to merge 66 commits from JacquesLucke/blender:temp-copy-on-write into main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.
11 changed files with 98 additions and 138 deletions
Showing only changes of commit ee675db8f6 - Show all commits

View File

@ -40,6 +40,7 @@ class AnonymousAttributeID {
std::string name_;
public:
AnonymousAttributeID();
virtual ~AnonymousAttributeID() = default;
StringRefNull name() const
@ -49,7 +50,7 @@ class AnonymousAttributeID {
virtual std::string user_name() const;
const blender::bCopyOnWrite &cow() const
const bCopyOnWrite &cow() const
{
return cow_;
}

View File

@ -48,8 +48,8 @@ class GeometryComponent;
*/
class GeometryComponent {
private:
blender::bCopyOnWrite cow_;
GeometryComponentType type_;
bCopyOnWrite cow_;
public:
GeometryComponent(GeometryComponentType type);
@ -68,16 +68,11 @@ class GeometryComponent {
/* The returned component should be of the same type as the type this is called on. */
virtual GeometryComponent *copy() const = 0;
const blender::bCopyOnWrite &cow() const
const bCopyOnWrite &cow() const
{
return cow_;
}
void cow_delete_self() const
{
delete this;
}
/* Direct data is everything except for instances of objects/collections.
* If this returns true, the geometry set can be cached and is still valid after e.g. modifier
* evaluation ends. Instances can only be valid as long as the data they instance is valid. */

View File

@ -4,6 +4,11 @@
namespace blender::bke {
AnonymousAttributeID::AnonymousAttributeID()
: cow_(1, this, [this](const bCopyOnWrite * /*cow*/) { MEM_delete(this); })
{
}
std::string AnonymousAttributeID::user_name() const
{
return this->name();

View File

@ -2361,21 +2361,31 @@ CustomData CustomData_shallow_copy_remove_non_bmesh_attributes(const CustomData
return dst;
}
static bCopyOnWrite *make_cow_for_array(const eCustomDataType type,
const void *data,
const int totelem)
{
return MEM_new<bCopyOnWrite>(__func__, 1, data, [type, totelem](const bCopyOnWrite *cow) {
free_layer_data(type, cow->data(), totelem);
MEM_delete(cow);
});
}
static void ensure_layer_data_is_mutable(CustomDataLayer &layer, const int totelem)
{
if (layer.data == nullptr) {
return;
}
if (layer.cow == nullptr) {
/* Can not be shared without cow data. */
return;
}
if (BLI_cow_is_shared(layer.cow)) {
if (layer.cow->is_shared()) {
const eCustomDataType type = eCustomDataType(layer.type);
const void *old_data = layer.data;
layer.data = copy_layer_data(type, old_data, totelem);
if (BLI_cow_user_remove(layer.cow)) {
free_layer_data(type, old_data, totelem);
BLI_cow_free(layer.cow);
}
layer.cow = BLI_cow_new(1);
layer.cow->user_remove_and_delete_if_last();
layer.cow = make_cow_for_array(type, layer.data, totelem);
}
}
@ -2384,13 +2394,28 @@ void CustomData_realloc(CustomData *data, const int old_size, const int new_size
BLI_assert(new_size >= 0);
for (int i = 0; i < data->totlayer; i++) {
CustomDataLayer *layer = &data->layers[i];
/* TODO: Just copy to the resized buffer directly instead of doing a COW copy here. */
ensure_layer_data_is_mutable(*layer, old_size);
const LayerTypeInfo *typeInfo = layerType_getInfo(layer->type);
const int64_t old_size_in_bytes = int64_t(old_size) * typeInfo->size;
const int64_t new_size_in_bytes = int64_t(new_size) * typeInfo->size;
layer->data = MEM_reallocN(layer->data, new_size_in_bytes);
void *new_layer_data = MEM_mallocN(new_size_in_bytes, __func__);
/* Copy or relocate data to new array. */
if (layer->cow && layer->cow->is_shared() && typeInfo->copy) {
typeInfo->copy(layer->data, new_layer_data, std::min(old_size, new_size));
}
else {
memcpy(new_layer_data, layer->data, std::min(old_size_in_bytes, new_size_in_bytes));
}
/* Remove ownership of old array */
if (layer->cow) {
layer->cow->user_remove_and_delete_if_last();
layer->cow = nullptr;
}
/* Take ownership of new array. */
layer->data = new_layer_data;
if (layer->data) {
layer->cow = make_cow_for_array(eCustomDataType(layer->type), layer->data, new_size);
}
if (new_size > old_size) {
/* Initialize new values for non-trivial types. */
@ -2431,9 +2456,7 @@ void CustomData_copy_without_data(const struct CustomData *source,
static void customData_free_layer__internal(CustomDataLayer *layer, const int totelem)
{
if (layer->anonymous_id != nullptr) {
if (layer->anonymous_id->cow().user_remove()) {
layer->anonymous_id->cow_delete_self();
}
layer->anonymous_id->cow().user_remove_and_delete_if_last();
layer->anonymous_id = nullptr;
}
const eCustomDataType type = eCustomDataType(layer->type);
@ -2443,12 +2466,7 @@ static void customData_free_layer__internal(CustomDataLayer *layer, const int to
}
}
else {
if (BLI_cow_user_remove(layer->cow)) {
if (layer->data) {
free_layer_data(type, layer->data, totelem);
}
BLI_cow_free(layer->cow);
}
layer->cow->user_remove_and_delete_if_last();
}
}
@ -2831,7 +2849,7 @@ static CustomDataLayer *customData_add_layer__internal(CustomData *data,
new_layer.data = layer_data_to_assign;
new_layer.cow = cow_to_assign;
if (new_layer.cow) {
BLI_cow_user_add(new_layer.cow);
new_layer.cow->user_add();
}
}
break;
@ -2839,7 +2857,7 @@ static CustomDataLayer *customData_add_layer__internal(CustomData *data,
}
if (new_layer.data != nullptr && new_layer.cow == nullptr) {
new_layer.cow = BLI_cow_new(1);
new_layer.cow = make_cow_for_array(type, new_layer.data, totelem);
}
new_layer.type = type;
@ -5101,10 +5119,7 @@ void CustomData_blend_write(BlendWriter *writer,
for (const CustomDataLayer &layer : layers_to_write) {
if (BLO_write_is_undo(writer) && layer.cow != nullptr) {
BLO_write_cow(
writer, layer.cow, layer.data, [type = eCustomDataType(layer.type), count](void *data) {
free_layer_data(type, data, count);
});
BLO_write_cow(writer, layer.cow);
continue;
}
switch (layer.type) {
@ -5225,11 +5240,13 @@ void CustomData_blend_read(BlendDataReader *reader, CustomData *data, const int
if (CustomData_verify_versions(data, i)) {
if (BLO_read_is_cow_data(reader, layer->data)) {
BLI_assert(layer->cow != nullptr);
BLI_cow_user_add(layer->cow);
layer->cow->user_add();
continue;
}
BLO_read_data_address(reader, &layer->data);
layer->cow = BLI_cow_new(1);
if (layer->data != nullptr) {
layer->cow = make_cow_for_array(eCustomDataType(layer->type), layer->data, count);
}
if (CustomData_layer_ensure_data_exists(layer, count)) {
/* Under normal operations, this shouldn't happen, but...
* For a CD_PROP_BOOL example, see T84935.

View File

@ -39,7 +39,8 @@ using blender::bke::Instances;
/** \name Geometry Component
* \{ */
GeometryComponent::GeometryComponent(GeometryComponentType type) : type_(type)
GeometryComponent::GeometryComponent(GeometryComponentType type)
: type_(type), cow_(1, this, [this](const bCopyOnWrite * /*cow*/) { delete this; })
{
}

View File

@ -6,6 +6,9 @@
* \ingroup bli
*/
#include <atomic>
#include <functional>
#include "BLI_compiler_attrs.h"
#include "BLI_utildefines.h"
#include "BLI_utility_mixins.hh"
@ -14,20 +17,7 @@
extern "C" {
#endif
typedef struct bCopyOnWrite {
int user_count;
} bCopyOnWrite;
bCopyOnWrite *BLI_cow_new(int user_count);
void BLI_cow_free(const bCopyOnWrite *cow);
void BLI_cow_init(const bCopyOnWrite *cow);
bool BLI_cow_is_shared(const bCopyOnWrite *cow);
bool BLI_cow_is_mutable(const bCopyOnWrite *cow);
void BLI_cow_user_add(const bCopyOnWrite *cow);
bool BLI_cow_user_remove(const bCopyOnWrite *cow) ATTR_WARN_UNUSED_RESULT;
typedef struct bCopyOnWrite bCopyOnWrite;
#ifdef __cplusplus
}
@ -35,13 +25,18 @@ bool BLI_cow_user_remove(const bCopyOnWrite *cow) ATTR_WARN_UNUSED_RESULT;
#ifdef __cplusplus
namespace blender {
struct bCopyOnWrite : blender::NonCopyable, blender::NonMovable {
private:
using DeleteFn = std::function<void(const bCopyOnWrite *cow)>;
mutable std::atomic<int> users_;
const void *data_;
DeleteFn delete_fn_;
struct bCopyOnWrite : public ::bCopyOnWrite, private NonCopyable, NonMovable {
public:
bCopyOnWrite()
bCopyOnWrite(const int initial_users, const void *data, DeleteFn delete_fn)
: users_(initial_users), data_(data), delete_fn_(std::move(delete_fn))
{
BLI_cow_init(this);
}
~bCopyOnWrite()
@ -49,27 +44,40 @@ struct bCopyOnWrite : public ::bCopyOnWrite, private NonCopyable, NonMovable {
BLI_assert(this->is_mutable());
}
const void *data() const
{
return data_;
}
bool is_shared() const
{
return BLI_cow_is_shared(this);
return users_.load(std::memory_order_relaxed) >= 2;
}
bool is_mutable() const
{
return BLI_cow_is_mutable(this);
return !this->is_shared();
}
void user_add() const
{
BLI_cow_user_add(this);
users_.fetch_add(1, std::memory_order_relaxed);
}
bool user_remove() const ATTR_WARN_UNUSED_RESULT
bool user_remove_and_delete_if_last() const
{
return BLI_cow_user_remove(this);
const int old_user_count = users_.fetch_sub(1, std::memory_order_relaxed);
BLI_assert(old_user_count >= 1);
const bool was_last_user = old_user_count == 1;
if (was_last_user) {
delete_fn_(this);
}
return was_last_user;
}
};
namespace blender {
template<typename T> class COWUser {
private:
T *data_ = nullptr;
@ -93,7 +101,7 @@ template<typename T> class COWUser {
~COWUser()
{
this->user_remove(data_);
this->user_remove_and_delete_if_last(data_);
}
COWUser &operator=(const COWUser &other)
@ -102,7 +110,7 @@ template<typename T> class COWUser {
return *this;
}
this->user_remove(data_);
this->user_remove_and_delete_if_last(data_);
data_ = other.data_;
this->user_add(data_);
return *this;
@ -114,7 +122,7 @@ template<typename T> class COWUser {
return *this;
}
this->user_remove(data_);
this->user_remove_and_delete_if_last(data_);
data_ = other.data_;
other.data_ = nullptr;
return *this;
@ -168,7 +176,7 @@ template<typename T> class COWUser {
void reset()
{
this->user_remove(data_);
this->user_remove_and_delete_if_last(data_);
data_ = nullptr;
}
@ -195,12 +203,10 @@ template<typename T> class COWUser {
}
}
static void user_remove(T *data)
static void user_remove_and_delete_if_last(T *data)
{
if (data != nullptr) {
if (data->cow().user_remove()) {
data->cow_delete_self();
}
data->cow().user_remove_and_delete_if_last();
}
}
};

View File

@ -17,57 +17,3 @@
/** \file
* \ingroup bli
*/
#include "atomic_ops.h"
#include "BLI_assert.h"
#include "BLI_copy_on_write.h"
#include "MEM_guardedalloc.h"
static int &get_counter(const bCopyOnWrite *cow)
{
BLI_assert(cow != nullptr);
return *const_cast<int *>(&cow->user_count);
}
bCopyOnWrite *BLI_cow_new(int user_count)
{
bCopyOnWrite *cow = MEM_cnew<bCopyOnWrite>(__func__);
cow->user_count = user_count;
return cow;
}
void BLI_cow_free(const bCopyOnWrite *cow)
{
BLI_assert(cow->user_count == 0);
MEM_freeN(const_cast<bCopyOnWrite *>(cow));
}
void BLI_cow_init(const bCopyOnWrite *cow)
{
get_counter(cow) = 1;
}
bool BLI_cow_is_mutable(const bCopyOnWrite *cow)
{
return !BLI_cow_is_shared(cow);
}
bool BLI_cow_is_shared(const bCopyOnWrite *cow)
{
return cow->user_count >= 2;
}
void BLI_cow_user_add(const bCopyOnWrite *cow)
{
atomic_fetch_and_add_int32(&get_counter(cow), 1);
}
bool BLI_cow_user_remove(const bCopyOnWrite *cow)
{
const int new_user_count = atomic_sub_and_fetch_int32(&get_counter(cow), 1);
BLI_assert(new_user_count >= 0);
const bool has_no_user_anymore = new_user_count == 0;
return has_no_user_anymore;
}

View File

@ -186,10 +186,7 @@ void BLO_write_string(BlendWriter *writer, const char *data_ptr);
* copied. A free-function has to be provided, which is called when the undo step is freed and no
* one else references the data anymore.
*/
void BLO_write_cow(BlendWriter *writer,
const bCopyOnWrite *cow,
const void *cow_data,
std::function<void(void *data)> free_data_fn);
void BLO_write_cow(BlendWriter *writer, const bCopyOnWrite *cow);
#endif
/**

View File

@ -30,12 +30,10 @@ typedef struct {
} MemFileChunk;
#ifdef __cplusplus
# include <functional>
# include "BLI_copy_on_write.h"
# include "BLI_map.hh"
struct MemFileCowStorage {
blender::Map<const void *, std::pair<const bCopyOnWrite *, std::function<void(void *)>>> map;
blender::Map<const void *, const bCopyOnWrite *> map;
~MemFileCowStorage();
};

View File

@ -58,10 +58,7 @@ void BLO_memfile_free(MemFile *memfile)
MemFileCowStorage::~MemFileCowStorage()
{
for (auto item : this->map.items()) {
if (BLI_cow_user_remove(item.value.first)) {
item.value.second(const_cast<void *>(item.key));
BLI_cow_free(item.value.first);
}
item.value->user_remove_and_delete_if_last();
}
}

View File

@ -1687,18 +1687,15 @@ bool BLO_write_is_undo(BlendWriter *writer)
return writer->wd->use_memfile;
}
void BLO_write_cow(BlendWriter *writer,
const bCopyOnWrite *cow,
const void *cow_data,
std::function<void(void *data)> free_data_fn)
void BLO_write_cow(BlendWriter *writer, const bCopyOnWrite *cow)
{
BLI_assert(writer->wd->use_memfile);
MemFile &memfile = *writer->wd->mem.written_memfile;
if (memfile.cow_storage == nullptr) {
memfile.cow_storage = MEM_new<MemFileCowStorage>(__func__);
}
if (memfile.cow_storage->map.add(cow_data, {cow, free_data_fn})) {
BLI_cow_user_add(cow);
if (memfile.cow_storage->map.add(cow->data(), cow)) {
Review

Maybe it doesn't matter, but maybe it would be reasonable to just create this once at the start of the writing process? It's likely to be used, and it would save the check for every write call. OTOH I'm sure this isn't a bottleneck!

Maybe it doesn't matter, but maybe it would be reasonable to just create this once at the start of the writing process? It's likely to be used, and it would save the check for every write call. OTOH I'm sure this isn't a bottleneck!
cow->user_add();
}
}