Callable: restore name in disengaged mode, add caching
#1446
+240
−167
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull request #1331 performed an optimization for
Callables, but changed observable semantics insafeguards-disengagedconfiguration: the callable name is removed. This may need some more discussion, as there's a small change that some game logic relies on the name (e.g. when stringifying aCallablein GDScript).This changes the approach, however performs different optimizations:
CowStr, akaCow<'static, str>instead ofGStringwhen constructing callablesGString(expensive) lazily, when actually usedGStringas well, to avoid recreating itI measured the
#[bench]performance compared to master in Release mode, with both balanced and disengaged safeguards. These are always minimum times.callable_callv_customcallable_callv_rust_fncallable_to_string_customcallable_to_string_rust_fnConclusion for this branch:
to_stringis equally fast or faster across the boardI would probably merge this due to quite a few other upcoming changes in
Callable, but we should maybe revisit perf in disengaged mode. Many people do indeed not care about the callable name, and I don't like that being the thing slowing things down. But I'm also not sure if disengaged should behave differently, this could set a dangerous precedent.It's quite possible that there are other optimization opportunities, or we make the name behavior more explicit.
cc @lyonbeckers