Separate Undo System from Read/Write File API
AbandonedPublic
Actions

Authored by Germano Cavalcante (mano-wii) on Jul 6 2021, 12:44 AM.

Details

Reviewers

Bastien Montagne (mont29)

Group Reviewers

Core

Summary

This is a raw patch that was made more for evaluating the idea.

Analyzing T88010: Memory consumption beyond defined limit when entering-exiting Edit mode, I realized that the logic of "compact array" used in edit-mode undo could be implemented for Mesh-type IDs and thus save memory usage.

But the current Undo System brings many obstacles to this implementation.

This is because "Global Undo" is very intertwined with the file writing/reading system.

Using file read/write functions for Undo is also misleading as, as far as I've seen, no files are written in undo steps.

This patch proposes to use specific functions for Undo thus separating what is for read/write files.

One downside is that a lot of code had to be duplicated, but there's probably still a lot of room for simplification.

Diff Detail

Repository

rB Blender

Branch

tmp-undo (branched from master)

Build Status

Buildable 15630
Build 15630: arc lint + arc unit

Event Timeline

Germano Cavalcante (mano-wii) requested review of this revision.Jul 6 2021, 12:44 AM

Germano Cavalcante (mano-wii) created this revision.

Harbormaster completed remote builds in B15630: Diff 39183.Jul 6 2021, 12:44 AM

It's unclear to me what your optimization idea is exactly. Can you explain how it would work, and why that requires duplicating this code?

There is already an abstraction layer for reading/writing memory. undofile.c has code to detect unchanged memory chunks. Potentially that system could be improved, to detect differences, compress data, etc. If we can do it at that generic level I think it would be better than e.g. adding some specialized optimization for meshes.

The file reading/writing is very hard to understand and fragile, duplicating that makes it harder to maintain. There would need to be a good motivation for it.

Perhaps the file reading/writing is complex because it is mixed in with the Undo system. Certainly there are many if/else conditions that are needed for file reading/writing but not undo and vice versa.
By separating the uses we can edit the Undo system without worrying about file reading/writing.

In D11818#304447, @Brecht Van Lommel (brecht) wrote:

It's unclear to me what your optimization idea is exactly. Can you explain how it would work, and why that requires duplicating this code?

The idea is basically to reuse the code seen in \editors\mesh\editmesh_undo.c. where the Bmesh is converted to Mesh and the CustomDatas are compared with mesh data of the previous undo, compressed and stored in a global variable.

Duplication is a first stage as it is still not very clear which part of the code is only needed for Undo and which part is only needed for reading/writing.

I have not looked too closely, but potentially the array store mechanism can be used for arbitrary memory blocks in undofile.c and does not need to be mesh specific. That would avoid code duplication and specialization, and optimize more than just meshes.

It's also not clear why entering/exiting edit mode as in T88010 should take so much memory in global undo. If you're just entering/exiting edit mode without making changes, you'd hope the mesh arrays would remain the same and get detected as identical in undofile.c. If that's not happening, it might be fixable and solve issues with more than just meshes.