memory.c cleanup and optimisation

[Isaac0-dev]

Low hanging fruit optimisations for the memory data structures in memory.c

Mostly generic performance improvements:

• Swapping calloc calls for malloc calls where it's safe
• Combined allocations into one large allocation for node + buffer
• Using realloc instead of full calloc + memcpy + free.

Revised growing_array_move

There's been a lot of discussion about this in the past, and I have decided this PR is a good opportunity to push this more formally.
You can find my final message on this topic here: #924 (comment).
The TLDR is that the confusion is mostly just a disagreement on what the parameter to should refer to. There are two ways one might imagine to should be:

• to should be relative to the array before any operation is performed.
• Or, to should be relative to the array after collapsing the removed block.

Currently without this PR, it uses the logic where to is relative to the array after collapsing the removed block.
This PR switches to the logic that to should be relative to the array before any operation is performed.
Because it's more intuitive. It's what a developer should be able to expect from an API perspective.
Developers shouldn't have to mentally simulate where to will be during the internal operation of the function.

I have added a version of the function that treats to as the destination index before any operations.
This version also has a new optimisation which uses a stack buffer for smaller, faster operations but uses dynamic memory when dealing with larger blocks.
The return type was also changed to boolean, which will allow callers to check if the move succeeded.

I've tested both the performance and the logic:

• Test logic: https://wandbox.org/permlink/W8HoIafGjvx96be6
• Test performance: https://quick-bench.com/q/gZfUFgh0pNEiIOU6slMu4xZPuEo

[PeachyPeachSM64]

Revised growing_array_move

Oh, come on... Why did you have to bring that up again?
This has to be a joke...

Currently without this PR, it uses the logic where to is relative to the array after collapsing the removed block.
This PR switches to the logic that to should be relative to the array before any operation is performed.
Because it's more intuitive. It's what a developer should be able to expect from an API perspective.
Developers shouldn't have to mentally simulate where to will be during the internal operation of the function.

That is absolutely WRONG. You reversed the logic AGAIN. Not once, not twice, but THREE times.
I even wrote an entire comment explaining why you're wrong (#924 (comment)), and visibly you still haven't get it right.
It really baffles me that while you were right on the very first implementation of the function that the logic was not intuitive (which assumed to was the destination on the modified array) (see comment: #924 (comment)), you claimed that the logic wasn't still right even after I modified it (commit: 9768e04, which happened after your comment), while proving you multiple times that the new logic used was clearly "to should be relative to the array before any operation is performed".

So, because it seems it's not clear enough, here we go again.

Let's take the array from your "Test logic": [A, B, C, D, E, F]
Let's use your "Move to End" example: from = 0, to = 4, count = 2
You claim that: to should be relative to the array before any operation is performed.

Let's index each element:

[ A, B, C, D, E, F ]
  ^  ^  ^  ^  ^  ^
  0  1  2  3  4  5

'to' has the value: 4. So, where is 'to'?
It should refer to the slot before any operation is performed: it's the slot that points to E.

[ A, B, C, D, E, F ]
              ^   
              4   

It means the moved chunck will be inserted right before E.
Which gives the following result:

[ A, B, C, D, E, F ]
  \___/      ^
    |________|

=>

[ C, D, A, B, E, F ]

Now, let's compare with your "expected" result: [ C, D, E, F, A, B ]
It's clearly not the same.

to should be relative to the array before any operation is performed, but your code doesn't seem to follow that logic, while the existing code does, and always did since this commit: 9768e04.

This version also has a new optimisation which uses a stack buffer for smaller, faster operations but uses dynamic memory when dealing with larger blocks.

This benchmark is not completely relevant. You gain a 80% speed boost only when the count is lower than the stack allocated buffer size. For all the other cases, it's actually slightly worse, because of the branch you introduce with the if statement.

Currently, this function is only used for SOC surfaces and surface nodes. Which has barely any usage. So this optimization trick doesn't even matter.
Even worse, with #1143, this function would not be used at all and would likely be removed.

---

I'm tired of arguing over the most pointless stuff and explaining it again and again.
Once again, I spent so much time writing this, hoping you will finally understand.
Please know that I don't and won't approve this. You can decide to ignore this message and merge it anyway, I don't care.
I won't care anymore.