Horrible Duplicate Performance

[mrjustaguy]

Tested versions

reproduced in 4.3 stable and 4.4 rc2

System information

Godot v4.3.stable - Windows 10.0.22631 - Vulkan (Forward+) - dedicated AMD Radeon RX 6600 (Advanced Micro Devices, Inc.; 32.0.12033.1030) - Intel(R) Core(TM) i3-10105F CPU @ 3.70GHz (8 Threads)

Issue description

Duplicate function is EXTREMELY expensive

in demo scene 100 duplicate calls take 30ms to execute, just to duplicate a simple standard material. having the material be local to scene and instancing the scene results in the same 30ms time to instantiate due to it calling duplicate for every one of the 100 scenes that each have a single local standard material

Steps to reproduce

open node3d scene

trigger ui left, ui up and ui right with a profiler
Observe massive spikes with the duplicate and local scene instancing not present with non local scene instancing

For me instancing and adding a child of the non local scene takes some 1.5ms for 100 instances, but some 30ms for instancing and adding a local scene, of which almost all the cost associated is due to the local scene internally calling duplicate for a single resource as can be demonstrated by just calling duplicate on the resource 100 times and getting the same performance cost.

Minimal reproduction project (MRP)

Duplicate.zip

[Zylann]

Workaround tip: I had to duplicate materials/shadermaterials a lot at runtime in my project, and found too that while duplicate is easy, it is far from optimal (notably due to the use of get_property_list and generic get/set usage internally). So instead I create a material manually and copy around relevant properties using the most direct APIs possible (properties, StringNames...). I also pool them to avoid re-creation. This is usually faster but requires more work. I haven't investigated Standard materials though, maybe there is another bottleneck here.

[clayjohn]

Looks like most of the time is in string operations in _validate_property()

Most of the remaining time is in the _validate_property() for the parent classes (GeometryInstance, Node3D, etc.)

I thought that _validate_property() isn't run on export, but it appears it is used in both export builds and in editor builds :/

[Calinou]

There's no reason to run _validate_property() in non-editor builds (or release builds at least), so we should make it a no-op.

[mrjustaguy]

_validate_property is also used in AudioSteamPlayer, which seems to exhibit very similar performance problems, though further testing is required for this one...

[Zylann]

_validate_property is documented as a way to alter existing properties (usually their flags), notably properties that were inherited (and therefore can't be changed otherwise). This means it can be used for things that are also not editor-specific. And even when games/tools use get_property_list (or indirectly, for example when saving objects), not running this method would end up in different behavior.
In my case, when making a custom procedural Cubemap class, I had to use it in order to prevent the _images property (inherited from TextureLayered) from being saved when the resource is saved, because like NoiseTexture, my texture is procedural and therefore this specific property shouldnt have the STORAGE flag.
I feel like this method ended up been more powerful than what it was added for? Because if it's supposed to be editor-only that's quite a big difference from what possibilities the documentation shows.
Personally I think there could be more efficient ways to do things (a bunch of use cases could simply modify flags once in the class data instead of having to check every property every time comparing strings etc; unless there is another way to do that I'm not aware of?)

[akien-mga]

Fixed (or at least mitigated) by #105907. Please let us know if the performance is still problematic and needs further profiling in 4.5 beta 1 or later.