WIP: Attributes: avoid unnecessary data copies by generalizing attribute unshare policy #119588

Jacques Lucke · 2024-03-17T11:46:28+01:00

Jacques Lucke commented

2024-03-17 11:46:28 +01:00

Currently, we sometimes do unnecessary data copies when getting write-access to an attributes. If the attribute was implicitly shared, it has to be copied internally before write-access can be provided. However, if the caller just overwrites all of the values without looking at the old values, the copy was unnecessary. A slightly more complex case is where the caller overwrites most, but not all values. In this case, it should only be necessary to copy a few old attribute values instead of all.

This patch solves a similar problem to #117179 but in a more general way. In some existing code we use *_for_write_only methods to indicate that the old values are not needed anymore. Both approaches don't support the more complex case where the attribute is partially overwritten.

This patch adds a new concept called "unshare policy". It is a function that makes a mutable version of shared data. The mutable version may be a full copy of the original data, a partial copy, or just uninitialized memory.

There are two related but separate unshare policy base types currently. One is specific to CustomData and one is more generally for arrays. This separation feels reasonable, because we want to use these unshare policies outside independent of CustomData. However, CustomData also has some less trivial storage formats (like for vertex groups) that can't easily be generalized currently.

The patch is not quite ready yet. It's still missing some documentation and open questions have to be resolved.

Open Questions:

Should we keep the *_for_write_only methods? Technically, they are redundant now but they seem to make the API usage more concise. Maybe it could be renamed to *_for_init_all to make the behavior more clear.

Currently, we sometimes do unnecessary data copies when getting write-access to an attributes. If the attribute was implicitly shared, it has to be copied internally before write-access can be provided. However, if the caller just overwrites all of the values without looking at the old values, the copy was unnecessary. A slightly more complex case is where the caller overwrites most, but not all values. In this case, it should only be necessary to copy a few old attribute values instead of all. This patch solves a similar problem to #117179 but in a more general way. In some existing code we use `*_for_write_only` methods to indicate that the old values are not needed anymore. Both approaches don't support the more complex case where the attribute is partially overwritten. This patch adds a new concept called "unshare policy". It is a function that makes a mutable version of shared data. The mutable version may be a full copy of the original data, a partial copy, or just uninitialized memory. There are two related but separate unshare policy base types currently. One is specific to `CustomData` and one is more generally for arrays. This separation feels reasonable, because we want to use these unshare policies outside independent of `CustomData`. However, `CustomData` also has some less trivial storage formats (like for vertex groups) that can't easily be generalized currently. ----- The patch is not quite ready yet. It's still missing some documentation and open questions have to be resolved. Open Questions: * Should we keep the `*_for_write_only` methods? Technically, they are redundant now but they seem to make the API usage more concise. Maybe it could be renamed to `*_for_init_all` to make the behavior more clear.

Jacques Lucke added 4 commits 2024-03-17 11:46:38 +01:00

53d835e9c2 initial unshare policy

b4de2ffddc add more general array unshare policy

2efa6a4e81 add unshare policy in more places

d7a1da6d69 use ignore-old-values policy

Jacques Lucke changed title from ~~Attributes: avoid unnecessary data copies by generalizing attribute unshare policy~~ to WIP: Attributes: avoid unnecessary data copies by generalizing attribute unshare policy

2024-03-17 11:52:48 +01:00

Jacques Lucke requested review from Hans Goudey 2024-03-17 12:06:04 +01:00

Iliya Katushenock commented

2024-03-22 17:53:29 +01:00

General solution for #117910 cases?

General solution for https://projects.blender.org/blender/blender/pulls/117910 cases?

Jacques Lucke commented

2024-03-23 08:56:35 +01:00

Not quite. In your case you still need access to the old array.

This pull request has changes conflicting with the target branch.

source/blender/blenkernel/intern/attribute_access.cc

View command line instructions.

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u attribute-unshare:JacquesLucke-attribute-unshare

git checkout JacquesLucke-attribute-unshare

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

WIP: Attributes: avoid unnecessary data copies by generalizing attribute unshare policy #119588

Checkout