Fix off-by-one in alloc_many unintentionally allocating fresh indices

[nickbabcock]

Objective

Currently there is an off-by-one bug in alloc_many. spawn_batch would allocate fresh indices instead of reusing freed ones when current_len == free.len().

I don't believe this bug exists on main, hence why this PR is targeting 0.18.1. Please let me know if there is a better way to land this change.

Testing

• The unit test covers the bug (ie: reverting the change will cause the test to fail).

[github-actions]

Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke!
You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-22785

If it's expected, please add the M-Deliberate-Rendering-Change label.

If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.

[alice-i-cecile]

Given that this is fixed on main but we can't cleanly cherrpick an existing fix I think this is the least bad way to do this.

[ElliottjPierce]

Yup! My bad.

Funnily enough, I actually made nearly the exact same mistake in the new allocator and had to fix it there too. Shows the importance of good unit tests I guess.

Thanks for finding and fixing this!

[kristoff3r]

@alice-i-cecile shouldn't the test have been added to main as well, or is it no longer relevant there? I tried it on main and it compiles fine, but it fails when run.

[ElliottjPierce]

@alice-i-cecile shouldn't the test have been added to main as well, or is it no longer relevant there? I tried it on main and it compiles fine, but it fails when run.

We have other tests on main that cover this now. The bug was in the new allocator, but I fixed that and added additional tests. The tests I added sort and dedupe instead of computing the intersection, but same thing really.

The reason this test fails on main is because we have a local_free queue that is always freed to before flushing that to the shared allocator. If you add a flush_freed() after each free in this test, it should work (but we have a problem if it doesn't.)

This local_free exists to reduce how often we need to use atomics on the shared allocator, and it results in a pretty nice performance improvement. But it does mean that tests that depend on entities being allocated in a particular order need to flush_freed().

See also:
#22658
#22672