Undo: support implicit-sharing in memfile undo step
This adds implicit sharing support for the `MemFile` undo-step. This decreases memory usage and increases performance. Implicit sharing allows the undo system to take (shared) ownership of some data. Previously, the data would always be serialized and compared to the previous undo-step. So this turns an O(n) operation into O(1) (in terms of memory usage and time). Read/write code that wants to make use of this has to use the new `BLO_read_shared` and `BLO_write_shared` functions respectively. Those either make use of implicit-sharing internally or do the "full" read/write based on a passed-in function. It seems possible to use the same API in the future to store shared data to .blend files. Improvements: * Much faster undo step creation in many cases by avoiding the majority data copies and equality checks. This fixes #98574. I found undo step creation and undo step decoding to be 2-5 times faster in some demo files from the blender website and in some production files from the Heist project. * Reduced memory usage when there is large data in `bmain`. For example, when loading the same highly subdivided mesh that I used in #106228 the memory usage is 1.03 GB now (compared to 1.62 GB in `main` currently). The main remaining copy of the data now is done by rendering code. * Some significant performance improvements were also measured for the new grease pencil type (#105540). There is one main downside of using implicit-sharing as implemented here: `MemFile` undo steps can't be written as .blend files anymore. This has a few consequences: * Auto-save becomes slower (up to 3x), because it can't just save the previous undo step anymore and does a normal save instead. This has been discussed in more detail here: https://devtalk.blender.org/t/remove-support-for-saving-memfile-undo-steps-as-blend-files-proposal/33544 It would be nice to work towards asynchronous auto-save to alleviate this problem. Some previous work has been done to reduce the impact of this change in41b10424c7andf0f304e240. This has been committed separately inefb511a76d. * Writing `quit.blend` has to do a normal file save now. So it's a bit slower too, but it's less of a problem in practice. * The `USE_WRITE_CRASH_BLEND` functionality does not work anymore. It doesn't seem to be used by anyone (removed ine90f5d03c4) There are also benefits to not writing `MemFile` from undo steps to disk. It allows us to more safely do undo-specific optimizations without risking corrupted .blend files. This is especially useful when we want to preserve forward compatibility in some cases. This requires converting data before writing the .blend files, but this conversion is not necessary for undo steps. Trying to implement this kind of optimization in the past has often lead to bugs (e.g.43b37fbc93). Another new problem is that it is harder to know the size of each undo step. Currently, a heuristic is used to approximate the memory usage, but better solutions could be found if necessary. Pull Request: https://projects.blender.org/blender/blender/pulls/106903
This commit is contained in:
@@ -1541,9 +1541,11 @@ void CurvesGeometry::blend_read(BlendDataReader &reader)
|
||||
CustomData_blend_read(&reader, &this->curve_data, this->curve_num);
|
||||
|
||||
if (this->curve_offsets) {
|
||||
BLO_read_int32_array(&reader, this->curve_num + 1, &this->curve_offsets);
|
||||
this->runtime->curve_offsets_sharing_info = implicit_sharing::info_for_mem_free(
|
||||
this->curve_offsets);
|
||||
this->runtime->curve_offsets_sharing_info = BLO_read_shared(
|
||||
&reader, &this->curve_offsets, [&]() {
|
||||
BLO_read_int32_array(&reader, this->curve_num + 1, &this->curve_offsets);
|
||||
return implicit_sharing::info_for_mem_free(this->curve_offsets);
|
||||
});
|
||||
}
|
||||
|
||||
BLO_read_list(&reader, &this->vertex_group_names);
|
||||
@@ -1569,7 +1571,14 @@ void CurvesGeometry::blend_write(BlendWriter &writer,
|
||||
CustomData_blend_write(
|
||||
&writer, &this->curve_data, write_data.curve_layers, this->curve_num, CD_MASK_ALL, &id);
|
||||
|
||||
BLO_write_int32_array(&writer, this->curve_num + 1, this->curve_offsets);
|
||||
if (this->curve_offsets) {
|
||||
BLO_write_shared(
|
||||
&writer,
|
||||
this->curve_offsets,
|
||||
sizeof(int) * (this->curve_num + 1),
|
||||
this->runtime->curve_offsets_sharing_info,
|
||||
[&]() { BLO_write_int32_array(&writer, this->curve_num + 1, this->curve_offsets); });
|
||||
}
|
||||
|
||||
BKE_defbase_blend_write(&writer, &this->vertex_group_names);
|
||||
}
|
||||
|
||||
@@ -5374,7 +5374,10 @@ void CustomData_blend_write(BlendWriter *writer,
|
||||
writer, CustomDataLayer, data->totlayer, data->layers, layers_to_write.data());
|
||||
|
||||
for (const CustomDataLayer &layer : layers_to_write) {
|
||||
blend_write_layer_data(writer, layer, count);
|
||||
const size_t size_in_bytes = CustomData_sizeof(eCustomDataType(layer.type)) * count;
|
||||
BLO_write_shared(writer, layer.data, size_in_bytes, layer.sharing_info, [&]() {
|
||||
blend_write_layer_data(writer, layer, count);
|
||||
});
|
||||
}
|
||||
|
||||
if (data->external) {
|
||||
@@ -5430,11 +5433,6 @@ static void blend_read_paint_mask(BlendDataReader *reader,
|
||||
static void blend_read_layer_data(BlendDataReader *reader, CustomDataLayer &layer, const int count)
|
||||
{
|
||||
BLO_read_data_address(reader, &layer.data);
|
||||
if (layer.data != nullptr) {
|
||||
/* Make layer data shareable. */
|
||||
layer.sharing_info = make_implicit_sharing_info_for_layer(
|
||||
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 #84935.
|
||||
@@ -5479,7 +5477,12 @@ void CustomData_blend_read(BlendDataReader *reader, CustomData *data, const int
|
||||
layer->sharing_info = nullptr;
|
||||
|
||||
if (CustomData_verify_versions(data, i)) {
|
||||
blend_read_layer_data(reader, *layer, count);
|
||||
layer->sharing_info = BLO_read_shared(reader, &layer->data, [&]() {
|
||||
blend_read_layer_data(reader, *layer, count);
|
||||
return layer->data ? make_implicit_sharing_info_for_layer(
|
||||
eCustomDataType(layer->type), layer->data, count) :
|
||||
nullptr;
|
||||
});
|
||||
i++;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -292,6 +292,7 @@ static void mesh_blend_write(BlendWriter *writer, ID *id, const void *id_address
|
||||
}
|
||||
}
|
||||
|
||||
const blender::bke::MeshRuntime *mesh_runtime = mesh->runtime;
|
||||
mesh->runtime = nullptr;
|
||||
|
||||
BLO_write_id_struct(writer, Mesh, id_address, &mesh->id);
|
||||
@@ -317,7 +318,12 @@ static void mesh_blend_write(BlendWriter *writer, ID *id, const void *id_address
|
||||
writer, &mesh->face_data, face_layers, mesh->faces_num, CD_MASK_MESH.pmask, &mesh->id);
|
||||
|
||||
if (mesh->face_offset_indices) {
|
||||
BLO_write_int32_array(writer, mesh->faces_num + 1, mesh->face_offset_indices);
|
||||
BLO_write_shared(
|
||||
writer,
|
||||
mesh->face_offset_indices,
|
||||
sizeof(int) * mesh->faces_num,
|
||||
mesh_runtime->face_offsets_sharing_info,
|
||||
[&]() { BLO_write_int32_array(writer, mesh->faces_num + 1, mesh->face_offset_indices); });
|
||||
}
|
||||
}
|
||||
|
||||
@@ -363,9 +369,11 @@ static void mesh_blend_read_data(BlendDataReader *reader, ID *id)
|
||||
mesh->runtime = new blender::bke::MeshRuntime();
|
||||
|
||||
if (mesh->face_offset_indices) {
|
||||
BLO_read_int32_array(reader, mesh->faces_num + 1, &mesh->face_offset_indices);
|
||||
mesh->runtime->face_offsets_sharing_info = blender::implicit_sharing::info_for_mem_free(
|
||||
mesh->face_offset_indices);
|
||||
mesh->runtime->face_offsets_sharing_info = BLO_read_shared(
|
||||
reader, &mesh->face_offset_indices, [&]() {
|
||||
BLO_read_int32_array(reader, mesh->faces_num + 1, &mesh->face_offset_indices);
|
||||
return blender::implicit_sharing::info_for_mem_free(mesh->face_offset_indices);
|
||||
});
|
||||
}
|
||||
|
||||
if (mesh->mselect == nullptr) {
|
||||
|
||||
@@ -125,6 +125,11 @@ class ImplicitSharingInfo : NonCopyable, NonMovable {
|
||||
return version_.load(std::memory_order_acquire);
|
||||
}
|
||||
|
||||
int strong_users() const
|
||||
{
|
||||
return strong_users_.load(std::memory_order_acquire);
|
||||
}
|
||||
|
||||
/**
|
||||
* 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.
|
||||
|
||||
@@ -33,6 +33,11 @@
|
||||
|
||||
#include "DNA_windowmanager_types.h" /* for eReportType */
|
||||
|
||||
#include "BLI_function_ref.hh"
|
||||
|
||||
namespace blender {
|
||||
class ImplicitSharingInfo;
|
||||
}
|
||||
struct BlendDataReader;
|
||||
struct BlendFileReadReport;
|
||||
struct BlendLibReader;
|
||||
@@ -182,6 +187,21 @@ void BLO_write_string(BlendWriter *writer, const char *data_ptr);
|
||||
|
||||
/* Misc. */
|
||||
|
||||
/**
|
||||
* Check if the data can be written more efficiently by making use of implicit-sharing. If yes, the
|
||||
* user count of the sharing-info is increased making the data immutable. The provided callback
|
||||
* should serialize the potentially shared data. It is only called when necessary.
|
||||
*
|
||||
* \param approximate_size_in_bytes: Used to be able to approximate how large the undo step is in
|
||||
* total.
|
||||
* \param write_fn: Use the #BlendWrite to serialize the potentially shared data.
|
||||
*/
|
||||
void BLO_write_shared(BlendWriter *writer,
|
||||
const void *data,
|
||||
size_t approximate_size_in_bytes,
|
||||
const blender::ImplicitSharingInfo *sharing_info,
|
||||
blender::FunctionRef<void()> write_fn);
|
||||
|
||||
/**
|
||||
* Sometimes different data is written depending on whether the file is saved to disk or used for
|
||||
* undo. This function returns true when the current file-writing is done for undo.
|
||||
@@ -245,6 +265,26 @@ void BLO_read_pointer_array(BlendDataReader *reader, void **ptr_p);
|
||||
|
||||
/* Misc. */
|
||||
|
||||
void blo_read_shared_impl(BlendDataReader *reader,
|
||||
void *data,
|
||||
const blender::ImplicitSharingInfo **r_sharing_info,
|
||||
blender::FunctionRef<const blender::ImplicitSharingInfo *()> read_fn);
|
||||
|
||||
/**
|
||||
* Check if there is any shared data for the given data pointer. If yes, return the existing
|
||||
* sharing-info. If not, call the provided function to actually read the data now.
|
||||
*/
|
||||
template<typename T>
|
||||
const blender::ImplicitSharingInfo *BLO_read_shared(
|
||||
BlendDataReader *reader,
|
||||
T **data_ptr,
|
||||
blender::FunctionRef<const blender::ImplicitSharingInfo *()> read_fn)
|
||||
{
|
||||
const blender::ImplicitSharingInfo *sharing_info;
|
||||
blo_read_shared_impl(reader, *data_ptr, &sharing_info, read_fn);
|
||||
return sharing_info;
|
||||
}
|
||||
|
||||
int BLO_read_fileversion_get(BlendDataReader *reader);
|
||||
bool BLO_read_requires_endian_switch(BlendDataReader *reader);
|
||||
bool BLO_read_data_is_undo(BlendDataReader *reader);
|
||||
|
||||
@@ -13,9 +13,22 @@
|
||||
#include "BLI_listbase.h"
|
||||
#include "BLI_map.hh"
|
||||
|
||||
namespace blender {
|
||||
class ImplicitSharingInfo;
|
||||
}
|
||||
struct GHash;
|
||||
struct Main;
|
||||
struct Scene;
|
||||
|
||||
struct MemFileSharedStorage {
|
||||
/**
|
||||
* Maps the data pointer to the sharing info that it is owned by.
|
||||
*/
|
||||
blender::Map<const void *, const blender::ImplicitSharingInfo *> map;
|
||||
|
||||
~MemFileSharedStorage();
|
||||
};
|
||||
|
||||
struct MemFileChunk {
|
||||
void *next, *prev;
|
||||
const char *buf;
|
||||
@@ -35,6 +48,11 @@ struct MemFileChunk {
|
||||
struct MemFile {
|
||||
ListBase chunks;
|
||||
size_t size;
|
||||
/**
|
||||
* Some data is not serialized into a new buffer because the undo-step can take ownership of it
|
||||
* without making a copy. This is faster and requires less memory.
|
||||
*/
|
||||
MemFileSharedStorage *shared_storage;
|
||||
};
|
||||
|
||||
struct MemFileWriteData {
|
||||
|
||||
@@ -4956,6 +4956,32 @@ void BLO_read_pointer_array(BlendDataReader *reader, void **ptr_p)
|
||||
*ptr_p = final_array;
|
||||
}
|
||||
|
||||
void blo_read_shared_impl(
|
||||
BlendDataReader *reader,
|
||||
void *data,
|
||||
const blender::ImplicitSharingInfo **r_sharing_info,
|
||||
const blender::FunctionRef<const blender::ImplicitSharingInfo *()> read_fn)
|
||||
{
|
||||
if (BLO_read_data_is_undo(reader)) {
|
||||
if (reader->fd->flags & FD_FLAGS_IS_MEMFILE) {
|
||||
UndoReader *undo_reader = reinterpret_cast<UndoReader *>(reader->fd->file);
|
||||
MemFile &memfile = *undo_reader->memfile;
|
||||
if (memfile.shared_storage) {
|
||||
/* Check if the data was saved with sharing-info. */
|
||||
if (const blender::ImplicitSharingInfo *sharing_info =
|
||||
memfile.shared_storage->map.lookup_default(data, nullptr))
|
||||
{
|
||||
/* Add a new owner of the data that is passed to the caller. */
|
||||
sharing_info->add_user();
|
||||
*r_sharing_info = sharing_info;
|
||||
return;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
*r_sharing_info = read_fn();
|
||||
}
|
||||
|
||||
bool BLO_read_data_is_undo(BlendDataReader *reader)
|
||||
{
|
||||
return (reader->fd->flags & FD_FLAGS_IS_MEMFILE);
|
||||
|
||||
@@ -25,6 +25,7 @@
|
||||
#include "DNA_listBase.h"
|
||||
|
||||
#include "BLI_blenlib.h"
|
||||
#include "BLI_implicit_sharing.hh"
|
||||
|
||||
#include "BLO_readfile.hh"
|
||||
#include "BLO_undofile.hh"
|
||||
@@ -45,9 +46,19 @@ void BLO_memfile_free(MemFile *memfile)
|
||||
}
|
||||
MEM_freeN(chunk);
|
||||
}
|
||||
MEM_delete(memfile->shared_storage);
|
||||
memfile->shared_storage = nullptr;
|
||||
memfile->size = 0;
|
||||
}
|
||||
|
||||
MemFileSharedStorage::~MemFileSharedStorage()
|
||||
{
|
||||
for (const blender::ImplicitSharingInfo *sharing_info : map.values()) {
|
||||
/* Removing the user makes sure shared data is freed when the undo step was its last owner. */
|
||||
sharing_info->remove_user_and_delete_if_last();
|
||||
}
|
||||
}
|
||||
|
||||
void BLO_memfile_merge(MemFile *first, MemFile *second)
|
||||
{
|
||||
/* We use this mapping to store the memory buffers from second memfile chunks which are not owned
|
||||
|
||||
@@ -91,6 +91,7 @@
|
||||
#include "BLI_blenlib.h"
|
||||
#include "BLI_endian_defines.h"
|
||||
#include "BLI_endian_switch.h"
|
||||
#include "BLI_implicit_sharing.hh"
|
||||
#include "BLI_link_utils.h"
|
||||
#include "BLI_linklist.h"
|
||||
#include "BLI_math_base.h"
|
||||
@@ -1824,6 +1825,33 @@ void BLO_write_string(BlendWriter *writer, const char *data_ptr)
|
||||
}
|
||||
}
|
||||
|
||||
void BLO_write_shared(BlendWriter *writer,
|
||||
const void *data,
|
||||
const size_t approximate_size_in_bytes,
|
||||
const blender::ImplicitSharingInfo *sharing_info,
|
||||
const blender::FunctionRef<void()> write_fn)
|
||||
{
|
||||
if (data == nullptr) {
|
||||
return;
|
||||
}
|
||||
if (BLO_write_is_undo(writer)) {
|
||||
MemFile &memfile = *writer->wd->mem.written_memfile;
|
||||
if (sharing_info != nullptr) {
|
||||
if (memfile.shared_storage == nullptr) {
|
||||
memfile.shared_storage = MEM_new<MemFileSharedStorage>(__func__);
|
||||
}
|
||||
if (memfile.shared_storage->map.add(data, sharing_info)) {
|
||||
/* The undo-step takes (shared) ownership of the data, which also makes it immutable. */
|
||||
sharing_info->add_user();
|
||||
/* This size is an estimate, but good enough to count data with many users less. */
|
||||
memfile.size += approximate_size_in_bytes / sharing_info->strong_users();
|
||||
return;
|
||||
}
|
||||
}
|
||||
}
|
||||
write_fn();
|
||||
}
|
||||
|
||||
bool BLO_write_is_undo(BlendWriter *writer)
|
||||
{
|
||||
return writer->wd->use_memfile;
|
||||
|
||||
Reference in New Issue
Block a user