Fix missing script time for some functions in profiler

[reach-satori]

This PR adds the (toggleable in settings) ability to profile non-gdscript functions , and makes it so the time spent in them no longer disappears

Since it's a pretty big pr and touches a lot of code, I'll briefly explain what the problem was and how I solved it:

Essentially, the script profile information is saved only in GDScriptFunction. But when something that's not a GDScriptFunction is called from there, the code assumed it would profile normally anyway and subtracted the time spent in it to get a self-time, and this wrong information caused the bug.

I've added an additional hashmap attribute to GDScriptFunction::Profile that instead saves the information on any function directly under it in the stack that has no capability to profile itself. In profiling_get_frame_data these sub-profiles then get converted to normal profile frames and sent off as usual.

The changes in ScriptLanguage and GDScriptLanguage are just to get the information on whether profiling is enabled or not from the settings to the VM, following the same mold of the profiler enabling/disabling. I thought this was necessary because collecting more information naturally slows down the execution during profiling.

Fixes #23715, fixes #40251, fixes #29049

[vnen]

Overall looks okay but I do have some nitpicks.

The major one is calling this "builtins" which I think isn't clear. In general they are called "native". I would prefer to use profiler_include_native_calls than profiler_include_builtins as the editor setting name for instance.

[vnen]

I don't see why this would be the case. They should still have a base Object which will always have a class name (unless they are freed or it's null). The only exception are calls on built-in types (Vector2, String, etc.) which don't have a base Object.

[reach-satori]

You're right, that was my mistake. I can't figure out a nice way to distinguish native from user GDScript code at this point in the VM without bigger changes. After thinking about it for a while and testing some things, the best I could find was:

if (!base_obj || !base_obj->get_script_instance() || !base_obj->get_script_instance()->has_method(*methodname))

But I don't know, there seem to be a ton of potential corner cases. Can you think of a scenario where this would fail? Or a decent alternative? I'll spend some more time testing this more thoroughly regardless.

[vnen]

I don't think there are any good solutions. At this point the VM does not really know who owns the function being called (whether a script or a native class). It only delegates to Variant which does the actual dynamic dispatch.

I now wonder if this profiling should be done by Variant instead of the GDScript VM.

[reach-satori]

As per @vnen's comment, there seems to be no clean way to do this properly without bigger changes that would probably require a proposal first (and/or would be over my head by quite a bit atm), but I think I managed to get some of the way there using ClassDB. As it is, according to my testing, this works as intended with straightforward native calls from GDScript, but fails in a few important cases.

Those cases are: call_deferred(), call(), and new(), because I couldn't find a way to find out if what they're calling ends up in native code or GDScript code. So I've left them unchanged and they'll miss time, same as the current profiler does for all native functions.
There might be others of the same kind I'm missing, or other cases where this approach fails that I couldn't find or think of.

In case someone eventually wants to look into it, here's also the small testing project I was using. It's essentially just a loop that calls one from a selection of function configurations, with a menu, so you can look at the profiler behaviour.
prof_proj.zip

Although, if it seems like nothing useful can come of this, feel free to close this PR.

[YuriSizov]

Please don't merge master into your feature branch. Prefer rebasing, so your PR only contains changes relevant to your work and nothing else.

See this documentation, if you need help with rebasing.

[adamscott]

Hi @reach-satori! Thanks for your first PR.
It seems that you still have problems with your merge, as it shows still 3,313 files changed.

Don't hesitate to join our developer chat! We could help you with git and your PR.

[reach-satori]

@adamscott
Sorry about that, just had a little bit of git trouble with the rebase, should be fine now

[adamscott]

Happy to see that you resolved your issue!
And the invitation to join our developer chat still stands!

[adamscott]

Reviewed in today's GDScript meeting. This PR seems fine, we don't see how else the job could be done. It would be better to improve the code based on this PR than not.

Thank you for your PR!

[AThousandShips]

Looks good otherwise!

[Faless]

Amazing work 🏆 !
I just have some nitpicks on the parameter name for profiling_set_save_native_calls, but it's looking great otherwise!

Thanks a lot for working on this! 💙

[YuriSizov]

Applied suggestions and rebased on your behalf.

[YuriSizov]

Thanks, and congrats on your first merged Godot contribution!