[4.x] Faster queue free

[lawnjelly]

Calling queue_free() for large numbers of siblings could previously be very slow, with the time taken rising exponentially with number of children. This looked partly due to ordered_remove from the child list and notifications.

This PR identifies objects that are nodes, and sorts the deletion queue so that children are deleted in reverse child order. This minimizes the costs of reordering.

Fixes #61929
Master version of #62444

[akien-mga]

Check the 3.x PR for more details on the rationale and implementation: #62444

[lawnjelly]

Looks like pos has been renamed to index in #67403 breaking CI. Will rebase soon, but the gist is the same.

[RandomShaper]

Since the set ob objects to be deleted no longer works in a FIFO way, I'd upgrade the namings to remove the queue term. That'd be even up to the user facing level, where, for instance, queue_free would become free_deferred, deferred_free or something similar.

In the PR for 3.x, at least I'd rename the internal variable names to match the renames that would happen in 4.x, but I'd keep the user-facing names as they are now.

[lawnjelly]

Have rebased and added the note to the queue_free() docs that the order is not guaranteed.

Since the set ob objects to be deleted no longer works in a FIFO way, I'd upgrade the namings to remove the queue term. That'd be even up to the user facing level, where, for instance, queue_free would become free_deferred, deferred_free or something similar.

We should probably bikeshed on this in a PR meeting at some point in the future - just in case there are differences of opinion on renaming. A lot of code uses queue_free() already, and I'm not sure the word queue warrants that it must be FIFO (although granted a queue often is).

[RandomShaper]

Why -2?

[lawnjelly]

It was probably -2 for historical reasons rather than -1 (for debugging).

The actual value (as long as it is constant) only matters in terms of the sorting relation with the nodes. If you wanted the objects to be deleted after the nodes for instance you might use a high number. The negative value ensures the objects will be deleted before the nodes, and is probably one of the reasons I used int32_t over uint32_t. (If we used 0 as the constant, then there is the possibility of a node deleted before the objects, not that it probably matters that much.)

[RandomShaper]

It's not only that 'queue' suggests FIFO, but also that Godot already has the concept of deferred for stuff that, vaguely, must happen 'not immediately, but very shortly'. If the engine was being written now for the first time and, say, we already had signals, deferred calls, etc., and then we were about to add object deletion as it is implemented in this PR, we wouldn't be introducing the queue terminology. It'd just be another case of deferred.

If this isn't still a strong reason to warrant the rename now, I agree that a PR meeting or proposal would work.

[YuriSizov]

So now that #75627 has been merged, and the 3.x counterpart of this PR, #62444, has also been merged should we close this one and also resolve #61929?

[lawnjelly]

So now that #75627 has been merged, and the 3.x counterpart of this PR, #62444, has also been merged should we close this one and also resolve #61929?

This PR can probably be closed - I'll test the performance of reduz PR first for comparison.

As for #61929 we could keep that open until #74672 or similar is merged (that's the 3.x kind of equivalent of #75627 ). Although the title of the issue is for slow deletion, the same problem exists for detaching / moving nodes, and that is unresolved as yet in 3.x. Alternatively I could create another issue just for detaching / moving, but they are so similar it seems fine to keep them together. In fact I'll retitle it to make more obvious. 👍

[lawnjelly]

Closing unless we have evidence of further problems after #75627 .