BMesh: Respect data alignment for attributes #109237

Joseph Eagar · 2023-06-22T12:54:50+02:00

Joseph Eagar commented

2023-06-22 12:54:50 +02:00

This PR modifies customData_update_offsets to respect
memory alignment when laying out BMesh attribute
blocks. While most consumer CPUs (and all x86s) support
nonaligned memory access it can lead to performance
degradation.

The alignment process is simple; large attributes
are laid out first and then small ones, and padding
bytes are added as necessary. Common vector types
are assumed to align to their base type (e.g.
CD_PROP_FLOAT3 has an alignment of 4).

In the future we should probably allocate BMesh attributes
separately in some kind of shared memory pool collection.
But for now this should improve performance.

This PR modifies `customData_update_offsets` to respect memory alignment when laying out BMesh attribute blocks. While most consumer CPUs (and all x86s) support nonaligned memory access it can lead to performance degradation. The alignment process is simple; large attributes are laid out first and then small ones, and padding bytes are added as necessary. Common vector types are assumed to align to their base type (e.g. CD_PROP_FLOAT3 has an alignment of 4). In the future we should probably allocate BMesh attributes separately in some kind of shared memory pool collection. But for now this should improve performance.

Joseph Eagar added 1 commit 2023-06-22 12:55:03 +02:00

Core: Respect data alignment for BMesh attributes 61b45ec8c0

This PR modifes customData_update_offsets to respect
memory alignment when laying out BMesh attribute
blocks. While most consumer CPUs (and all x86s) support
nonaligned memory access it can lead to performance
degredation.

The alignment process is simple; large attributes
are layed out first and then small ones, and padding
bytes are added as necessary.  Common vector types
are assumed to align to their base type (e.g.
CD_PROP_FLOAT3 has an alignment of 4).

In the future we should probably allocate BMesh attributes
seperately in some kind of shared memory pool collection.
But for now this should improve performance.

Hans Goudey reviewed 2023-06-22 13:19:12 +02:00

Hans Goudey left a comment

If the goal here is improve performance, the PR should probably include some numbers to show the impact. Other than that the code looks reasonable, though now the function does get slower as we add more custom data types.

source/blender/blenkernel/intern/customdata.cc Outdated

						
				@ -61,6 +62,7 @@

				/* only for customdata_data_transfer_interp_normal_normals */

				#include "data_transfer_intern.h"

				using blender::Array;

Hans Goudey commented

2023-06-22 13:19:10 +02:00

Unnecessary change

JosephEagar marked this conversation as resolved

source/blender/blenkernel/intern/customdata.cc

						
				@ -2541,1 +2543,4 @@

				static int customData_get_alignment(eCustomDataType type)

				{

				  /* Handle array types. */

Hans Goudey commented

2023-06-22 13:16:19 +02:00

This comment isn't quite right, since CD_PROP_BYTE_COLOR is also an array type. Maybe "Handle float array types"?

This comment isn't quite right, since `CD_PROP_BYTE_COLOR` is also an array type. Maybe "Handle float array types"?

Joseph Eagar commented

2023-06-22 13:39:20 +02:00

I has an if statement for that. I admit the comment isn't clear, it applies to the three if statements (for 4,2,1 alignments).

Hans Goudey commented

2023-06-22 13:47:51 +02:00

Ah I see. I don't think the comment is necessary honestly

JosephEagar marked this conversation as resolved

source/blender/blenkernel/intern/customdata.cc Outdated

						
				@ -2545,0 +2594,4 @@

				  int max_alignment = 1;

				  int aligns[] = {8, 4, 2, 1};

Hans Goudey commented

2023-06-22 13:19:01 +02:00

std::array should work here, a bit nicer IMO

`std::array` should work here, a bit nicer IMO

JosephEagar marked this conversation as resolved

source/blender/blenkernel/intern/customdata.cc Outdated

						
				@ -2548,1 +2600,3 @@

				    typeInfo = layerType_getInfo(eCustomDataType(data->layers[i].type));

				  for (int i = 0; i < ARRAY_SIZE(aligns) + 1; i++) {

				    for (int j = 0; j < data->totlayer; j++) {

				      CustomDataLayer *layer = data->layers + j;