Use `increment` instead of `conditional_increment` for `SafeRefCount`

[Ivorforce]

Describe the project you are working on

Godot internals.

Describe the problem or limitation you are having in your project

I was looking at Godot's SafeRefCount implementation, which is used for all objects dealing with refcounts (RefCounted, Array...).

The implementation has an interesting quirk it has inherited from the very first Open Source godot:

It refuses to reference a SafeRefCount with 0 referrents.

The quirk is interesting for one main reason: If SafeRefCount decreases to 0, most¹ owners will self-destruct, taking SafeRefCount down with it. If the owner self-destructs, it is illegal to still hold a reference to it. Although well-meaning, SafeRefCount::ref will never return false in any legal scenario of self-destructing owners.

To account for this behavior, workarounds are made.
For example, RefCounted starts with a RefCount of 1, because a 0-refcount indicates the object is already destructed. To compensate for the unaccounted for reference, it will ignore the first refcount increment, instead using another SafeRefCount (refcount_init) to check if it was the first. This workaround costs observable performance, adding at least 3% delay to function calls in GDScript².

Describe the feature / enhancement and how it helps to overcome the problem or limitation

SafeRefCounted::ref should return true even if the previous refcount was 0. It should not judge in what state the refcount was previously. This behavior would mirror the way std::shared_ptr is implemented (at least on my system)³.

The change would make the behavior simpler and faster for all SafeRefCount users I've seen through the Godot engine.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Implement SafeRefCount::ref as such:

_ALWAYS_INLINE_ void ref() {
	count.increment();
}

Then, fix all compiler errors, start RefCounted::refcount at 0, and remove init_ref and refcount_init. Since the false case should never have been reached anyway, this should not result in more problems beyond compiler errors (although we all know there will be at least some...).

If this enhancement will not be used often, can it be worked around with a few lines of script?

No.

Is there a reason why this should be core and not an add-on in the asset library?

It's core.

Footnotes

1. An example of an exception: RefCounted refuses to self-destruct if any of its bindings refuse to self-destruct. In this case, the object exists in an odd half-zombie state, in which its refcount is 0 and therefore refuses to be re-referenced godot-side anymore until eventually destructed. In this case, allowing a re-reference would be correct. This situation can better be generalized to SafeRefCount making too many assumptions about not being allowed to increment from 0. ↩

2. Measured here. ↩

3. Although notably, std::shared_ptr copies never return false, even if the max refcount is reached, likely because 2^32 is such an absurdly high amount of refs you can just assume it will never be reached in any real scenario⁴. ↩

4. And in a way, Godot's do too, because reference return values are frequently (transitively) ignored out of convenience... ↩