Repository: godotengine/godot (language: cpp)

Issue:
Excessive StringName allocation when subclassing AnimationTree
### Tested versions

 - Reproducible in: v4.4.stable.mono.official [4c311cbee]

### System information

Godot v4.4.stable.mono - Windows 11 (build 26100) - Multi-window, 3 monitors - Vulkan (Forward+) - dedicated NVIDIA GeForce GTX 1080 (NVIDIA; 32.0.15.6109) - 13th Gen Intel(R) Core(TM) i5-13600KF (20 threads)

### Issue description

When subclassing the AnimationTree class, animations starts excessively generating `StringName`'s. If it isn't subclassed, this does not happen. This excess causes noticable stutters when the memory is reclaimed.

Here's a graph where I don't subclass the AnimationTree

https://github.com/user-attachments/assets/b344ad6f-963b-4e7e-8703-8ebfe1a72942

Here's a graph where I subclass the AnimationTree

https://github.com/user-attachments/assets/ef809afa-b9a1-4dd1-ac14-2d624333f341

Both cases are doing the same updates.

### Steps to reproduce

1. Create a custom class inheriting from AnimationTree
2. Give it a method
3. Have the method update parameters

Repro is attached
1. Run the project (non-normal)
2. Profile memory usage

### Minimal reproduction project (MRP)

Bugged (with subclass): [StringNameLeak.zip](https://github.com/user-attachments/files/19263124/StringNameLeak.zip)
Normal (without subclass): [StringNameLeak (normal).zip](https://github.com/user-attachments/files/19263181/StringNameLeak.normal.zip)


Discussion / hints:
Thank you for the detailed report and the minimal reproduction projects. This is definitely something we need to look into.

It looks like the cause is calling the `Set` method. When the method is called on a Node/Resource that has a script attached, it will also call the `_set` virtual method implemented by the script. That's why the behavior is different when you subclass `AnimationTree`.

In C#, calling the `_set` virtual method means we end up allocating a new `StringName` instance to pass it to the `_Set` virtual method:

https://github.com/godotengine/godot/blob/0028fd625e2c5b202e204bc12828cbca043213d3/modules/mono/glue/GodotSharp/GodotSharp/Core/Bridge/CSharpInstanceBridge.cs#L62-L67

This is unfortunate because your script doesn't actually override `_Set` so it'd be great if we could avoid it. We may be able to check if the C# instance overrides `_Set` with reflection, but that may break trimming scenarios (i.e. NativeAOT).

You may be able to avoid `Set` entirely if you can access the `AnimationNode` and call `SetParameter` instead, maybe we can expose a `SetParameter` method in `AnimationTree` or `AnimationMixer` that bypasses the `_set` virtual method.
Looks like I can access `AnimationTree.TreeRoot` which has a SetParameter method, so that should do the trick for now. I appreciate the suggestion.
I tested using the `AnimationTree.TreeRoot` and then using SetParameter, but I get the following error:
```
ERROR: Parameter "process_state" is null.
   at: set_parameter (scene/animation/animation_tree.cpp:73)
```
I'll just hold off subclassing for now, and then await a potential fix.
Can we give an alternative lower-level API for this special case (`_get`, `_set`) to enable the developer to cache the allocations?
@Delsin-Yu I'm not sure what you mean, the developer is already using a cached `StringName` in this case:

```csharp
public partial class SpecialAnimationTree : AnimationTree
{
    private static readonly StringName IdleBlendPosition = "parameters/Idle/blend_position";

    public void Update(...)
    {
        Set(IdleBlendPosition, ...);
    }
}
```

Did you mean providing a new API that calls `set` without calling the virtual `_set` method?
> Did you mean providing a new API that calls set without calling the virtual _set method?

Actually, after checking the source code, I think it is possible to optimize away the StringName allocations in `CSInstanceBridge.set` and `CSInstanceBridge.get` without introduces new API (only two publicly visible boolean flags), we can use the source generator to detect whether the user has overridden this method and generate an override boolean property getter to store this information for runtime evaluation.

```csharp
[UnmanagedCallersOnly]
internal static unsafe godot_bool Set(IntPtr godotObjectGCHandle, godot_string_name* name, godot_variant* value)
{
    try
    {
        var godotObject = (GodotObject)GCHandle.FromIntPtr(godotObjectGCHandle).Target;

        if (godotObject == null)
            throw new InvalidOperationException();

        if (godotObject.SetGodotClassPropertyValue(CustomUnsafe.AsRef(name), CustomUnsafe.AsRef(value)))
        {
            return godot_bool.True;
        }

        if (!godotObject.HasCustomSetMethod) // Overriden by source generator in user scripts
        {
            return godot_bool.False;
        }

        var nameManaged = StringName.CreateTakingOwnershipOfDisposableValue(
            NativeFuncs.godotsharp_string_name_new_copy(CustomUnsafe.AsRef(name)));

        Variant valueManaged = Variant.CreateCopyingBorrowed(*value);

        return godotObject._Set(nameManaged, valueManaged).ToGodotBool();
    }
    catch (Exception e)
    {
        ExceptionUtils.LogException(e);
        return godot_bool.False;
    }
}
```

Fix patch:
diff --git a/modules/mono/glue/GodotSharp/GodotSharp/Core/Bridge/CSharpInstanceBridge.cs b/modules/mono/glue/GodotSharp/GodotSharp/Core/Bridge/CSharpInstanceBridge.cs
index 7b6b35b68fe3..d17b7327822c 100644
--- a/modules/mono/glue/GodotSharp/GodotSharp/Core/Bridge/CSharpInstanceBridge.cs
+++ b/modules/mono/glue/GodotSharp/GodotSharp/Core/Bridge/CSharpInstanceBridge.cs
@@ -59,6 +59,11 @@ internal static unsafe godot_bool Set(IntPtr godotObjectGCHandle, godot_string_n
                     return godot_bool.True;
                 }
 
+                if (!godotObject.HasGodotClassMethod(GodotObject.MethodName._Set.NativeValue.DangerousSelfRef))
+                {
+                    return godot_bool.False;
+                }
+
                 var nameManaged = StringName.CreateTakingOwnershipOfDisposableValue(
                     NativeFuncs.godotsharp_string_name_new_copy(CustomUnsafe.AsRef(name)));
 
@@ -107,6 +112,11 @@ internal static unsafe godot_bool Get(IntPtr godotObjectGCHandle, godot_string_n
                     return godot_bool.True;
                 }
 
+                if (!godotObject.HasGodotClassMethod(GodotObject.MethodName._Get.NativeValue.DangerousSelfRef))
+                {
+                    return godot_bool.False;
+                }
+
                 var nameManaged = StringName.CreateTakingOwnershipOfDisposableValue(
                     NativeFuncs.godotsharp_string_name_new_copy(CustomUnsafe.AsRef(name)));
 


Test patch: