Page MenuHome

Geometry: Avoid unnecessary initialization when resizing data arrays
ClosedPublic

Authored by Hans Goudey (HooglyBoogly) on Aug 31 2022, 2:20 AM.

Details

Summary

When resizing mesh and curves attribute storage, avoid initializing the
new memory for basic types. Also, avoid skipping "no free" layers; all
layers should be reallocated to the new size since they may be accessed.

The semantics introduced in 25237d2625078c6d1 are essential for this
change, because otherwise we don't have a way to construct non-trivial
types in the new memory.

In a basic test of the extrude node, I observed a performance
improvement of about 30%, from 55ms to 42ms.

The commit also includes utilities I found helpful for the change:
an attribute API function to retrieve a layer as a span without creating
it, and more functions for generic spans to mirror the typed versions.
I'll commit these parts separately though.

Diff Detail

Repository
rB Blender

Event Timeline

Jacques Lucke (JacquesLucke) requested changes to this revision.Aug 31 2022, 12:07 PM
Jacques Lucke (JacquesLucke) added inline comments.
source/blender/blenkernel/intern/customdata.cc
2420

Just removing this is not quite right, because MEM_reallocN may free the old layer->data, but it is still referenced somewhere else.

2420

The multiplication is probably done before the conversion from int to int64_t, which probably isn't what you've intended.

source/blender/blenkernel/intern/mesh.cc
2177

Are you sure that the new elements are initialized somewhere? Previously, they were zeroed, but that doesn't seem to be the case anymore. Same in all other places where CustomData_realloc is used.
Note that it might not be enough to initialize all attributes, all other custom data layers have to be initialized as well.

source/blender/blenlib/BLI_generic_span.hh
104

const int64_t

This revision now requires changes to proceed.Aug 31 2022, 12:07 PM
Hans Goudey (HooglyBoogly) marked 2 inline comments as done.
  • Fix initialization in extrude node
  • Some simplification and cleanup
  • Asserts in other areas to make assumptions about geometry "emptyness" more valid
  • Deal with "NOFREE" flag properly
source/blender/blenkernel/intern/customdata.cc
2420

Oops, of course, right. I'll do a new allocation and copy if the array is shared.

source/blender/blenkernel/intern/mesh.cc
2177

Here they are initialized in CustomData_copy_data above.
In the other areas, I made sure that the users of the realloc function initialized values if they had too. Luckily the function isn't used that much.

Hans Goudey (HooglyBoogly) marked an inline comment as done.Sep 1 2022, 8:03 PM
source/blender/blenkernel/BKE_customdata.h
181–182

This should document how the new data is initialized.

source/blender/nodes/geometry/nodes/node_geo_extrude_mesh.cc
97

Why did you remove CustomData_duplicate_referenced_layers?

Hans Goudey (HooglyBoogly) marked 3 inline comments as done.

Add information about initialization to comment

source/blender/nodes/geometry/nodes/node_geo_extrude_mesh.cc
97

CustomData_realloc now handles the CD_FLAG_NOFREE layers properly, so it's unnecessary.

This revision is now accepted and ready to land.Sep 12 2022, 5:13 PM