PyAPI: Enable MeshPolygon.loop_start raw array access #114262

Thomas Barlow · 2023-10-30T05:38:57+01:00

Thomas Barlow commented

2023-10-30 05:38:57 +01:00

This patch replaces MeshPolygon.loop_start's use of
RNA_def_property_int_funcs with RNA_def_property_int_sdna to enable raw
array access.

RNA_def_property_int_funcs disables the PROP_INTERN_RAW_ACCESS flag,
which forces rna_raw_access to fall back to its slower loop when using
bpy_prop_collection.foreach_get/foreach_set from Python.

Because MeshPolygon uses the MIntProperty struct and loop_start accesses
the "i" field of MIntProperty directly, it can use
RNA_def_property_int_sdna like other props that directly access the "i"
field of a StructRNA using the MIntProperty struct.

This then speeds up the Python API bpy_prop_collection.foreach_get/
foreach_set access of MeshPolygon.loop_total to the same speeds as
generic int attributes.

Given a mesh with 98304 faces:

foreach_get with a compatible buffer object:

Face int attribute: ~0.23ms
MeshPolygon.loop_start (before): ~1.8ms
MeshPolygon.loop_start (after): ~0.23ms

foreach_get with a Python list:

Face int attribute: ~2.2ms
MeshPolygon.loop_start (before): ~3.4ms
MeshPolygon.loop_start (after): ~2.2ms

foreach_set with a compatible buffer object:

Face int attribute: ~0.25ms
MeshPolygon.loop_start (before): ~2.3ms
MeshPolygon.loop_start (after): ~0.25ms

foreach_set with a Python list:

Face int attribute: ~1.1ms
MeshPolygon.loop_start (before): ~3.1ms
MeshPolygon.loop_start (after): ~1.1ms

Profiling some Python addon code has showed me that some props are slower with foreach_get/foreach_set than I expected, so I have been investigating and this PR serves as my result for MeshPolygon.loop_start.

After seeing that MeshPolygon was using the MIntProperty struct I compared it against other StructRNA using MIntProperty to try and find out why the loop_start prop was failing the itemprop->flag_internal & PROP_INTERN_RAW_ACCESS check in RNA_property_collection_raw_array. Changing loop_start to use RNA_def_property_int_sdna like the other StructRNA seems to have done the job.

I'm not super familiar with the terminology surrounding Blender's C code, but the commit message is my best guess given my current knowledge.

If there are plans to move MeshPolygon.loop_start to its own prop collection similar to the recent changes to MeshLoop.normal -> Mesh.corner_normals, then this PR can probably be discarded.

This patch replaces MeshPolygon.loop_start's use of RNA_def_property_int_funcs with RNA_def_property_int_sdna to enable raw array access. RNA_def_property_int_funcs disables the PROP_INTERN_RAW_ACCESS flag, which forces rna_raw_access to fall back to its slower loop when using bpy_prop_collection.foreach_get/foreach_set from Python. Because MeshPolygon uses the MIntProperty struct and loop_start accesses the "i" field of MIntProperty directly, it can use RNA_def_property_int_sdna like other props that directly access the "i" field of a StructRNA using the MIntProperty struct. This then speeds up the Python API bpy_prop_collection.foreach_get/ foreach_set access of MeshPolygon.loop_total to the same speeds as generic int attributes. Given a mesh with 98304 faces: foreach_get with a compatible buffer object: - Face int attribute: ~0.23ms - MeshPolygon.loop_start (before): ~1.8ms - MeshPolygon.loop_start (after): ~0.23ms foreach_get with a Python list: - Face int attribute: ~2.2ms - MeshPolygon.loop_start (before): ~3.4ms - MeshPolygon.loop_start (after): ~2.2ms foreach_set with a compatible buffer object: - Face int attribute: ~0.25ms - MeshPolygon.loop_start (before): ~2.3ms - MeshPolygon.loop_start (after): ~0.25ms foreach_set with a Python list: - Face int attribute: ~1.1ms - MeshPolygon.loop_start (before): ~3.1ms - MeshPolygon.loop_start (after): ~1.1ms --- Profiling some Python addon code has showed me that some props are slower with foreach_get/foreach_set than I expected, so I have been investigating and this PR serves as my result for MeshPolygon.loop_start. After seeing that MeshPolygon was using the MIntProperty struct I compared it against other StructRNA using MIntProperty to try and find out why the loop_start prop was failing the `itemprop->flag_internal & PROP_INTERN_RAW_ACCESS` check in RNA_property_collection_raw_array. Changing loop_start to use RNA_def_property_int_sdna like the other StructRNA seems to have done the job. I'm not super familiar with the terminology surrounding Blender's C code, but the commit message is my best guess given my current knowledge. If there are plans to move MeshPolygon.loop_start to its own prop collection similar to the recent changes to MeshLoop.normal -> Mesh.corner_normals, then this PR can probably be discarded.

Thomas Barlow added 1 commit 2023-10-30 05:39:12 +01:00

buildbot/vexp-code-patch-coordinator Build done. Details

d047ac0ffd PyAPI: Enable MeshPolygon.loop_start raw array access

This patch replaces MeshPolygon.loop_start's use of
RNA_def_property_int_funcs with RNA_def_property_int_sdna to enable raw
array access.

RNA_def_property_int_funcs disables the PROP_INTERN_RAW_ACCESS flag,
which forces rna_raw_access to fall back to its slower loop when using
bpy_prop_collection.foreach_get/foreach_set from Python.

Because MeshPolygon uses the MIntProperty struct and loop_start accesses
the "i" field of MIntProperty directly, it can use
RNA_def_property_int_sdna like other props that directly access the "i"
field of a StructRNA using the MIntProperty struct.

This then speeds up the Python API bpy_prop_collection.foreach_get/
foreach_set access of MeshPolygon.loop_total to the same speeds as
generic int attributes.

Given a mesh with 98304 faces:

foreach_get with a compatible buffer object:
- Face int attribute: ~0.23ms
- MeshPolygon.loop_start (before): ~1.8ms
- MeshPolygon.loop_start (after): ~0.23ms
foreach_get with a Python list:
- Face int attribute: ~2.2ms
- MeshPolygon.loop_start (before): ~3.4ms
- MeshPolygon.loop_start (after): ~2.2ms

foreach_set with a compatible buffer object:
- Face int attribute: ~0.25ms
- MeshPolygon.loop_start (before): ~2.3ms
- MeshPolygon.loop_start (after): ~0.25ms
foreach_set with a Python list:
- Face int attribute: ~1.1ms
- MeshPolygon.loop_start (before): ~3.1ms
- MeshPolygon.loop_start (after): ~1.1ms

Hans Goudey commented

2023-10-30 08:20:16 +01:00

Nice improvement, thanks for working on it!

If there are plans to move MeshPolygon.loop_start to its own prop collection similar to the recent changes to MeshLoop.normal -> Mesh.corner_normals, then this PR can probably be discarded.

It may be nicer to expose Mesh.face_offsets (finishig #109819) and deprecate the .polygons property longer term. There's no pressing reason to do that, but if we just "deprecate" the old properties but don't plan on removing them for a long time, it doesn't really have a downside either. What do you think about that?

Nice improvement, thanks for working on it! >If there are plans to move MeshPolygon.loop_start to its own prop collection similar to the recent changes to MeshLoop.normal -> Mesh.corner_normals, then this PR can probably be discarded. It may be nicer to expose `Mesh.face_offsets` (finishig #109819) and deprecate the `.polygons` property longer term. There's no pressing reason to do that, but if we just "deprecate" the old properties but don't plan on removing them for a long time, it doesn't really have a downside either. What do you think about that?

Thomas Barlow commented

2023-10-30 16:31:30 +01:00

It may be nicer to expose Mesh.face_offsets (finishig #109819) and deprecate the .polygons property longer term. There's no pressing reason to do that, but if we just "deprecate" the old properties but don't plan on removing them for a long time, it doesn't really have a downside either. What do you think about that?

Yeah, that's a good point, I hadn't really thought about that even if the old properties become deprecated in the short term, they will still exist in the longer term to maintain compatibility with existing Python scripts.

I've also now seen that Mesh.from_pydata uses foreach_set with MeshPolygon.loop_start., so this PR would affect a wide area due to the large amount of use of Mesh.from_pydata in addons.

> It may be nicer to expose `Mesh.face_offsets` (finishig #109819) and deprecate the `.polygons` property longer term. There's no pressing reason to do that, but if we just "deprecate" the old properties but don't plan on removing them for a long time, it doesn't really have a downside either. What do you think about that? Yeah, that's a good point, I hadn't really thought about that even if the old properties become deprecated in the short term, they will still exist in the longer term to maintain compatibility with existing Python scripts. I've also now seen that Mesh.from_pydata uses foreach_set with MeshPolygon.loop_start., so this PR would affect a wide area due to the large amount of use of Mesh.from_pydata in addons.

Hans Goudey approved these changes 2023-10-30 16:55:31 +01:00

Hans Goudey left a comment

Ah, I actually didn't look at the code yet, this seems good to me. Even if we have face_offsets, making code simpler and faster is just nice!

Ah, I actually didn't look at the code yet, this seems good to me. Even if we have `face_offsets`, making code simpler and faster is just nice!

Hans Goudey commented

2023-10-30 16:55:40 +01:00

@blender-bot build

Hans Goudey merged commit adc79ac8d8 into main

2023-10-30 21:03:44 +01:00

Hans Goudey referenced this issue from a commit

2023-10-30 21:03:45 +01:00

PyAPI: Enable MeshPolygon.loop_start raw array access

Thomas Barlow deleted branch meshpolygon_loop_start_raw_access

2024-01-09 23:05:25 +01:00

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

PyAPI: Enable MeshPolygon.loop_start raw array access #114262