Optimize Entities::reserve_entity to reduce memory and atomics

[mjhostet]

TL;DR: this cuts in half the number of atomic ops per
Entities::reserve_entity and also cuts in half the memory
overhead related to Entity freelisting and allocation.

Entities maintained two tables: reserved and free, each
with one slot for every possible Entity. This way every entity could
be freelisted or reserved without needing more memory. Combined,
these tables took 2*meta.len() slots.

The key observation is that any entity can only be in one table or the
other, but not both, so we actually only need meta.size() slots total,
i.e. half the memory. One way to take advantage of this would be to
have the freelist grow "up" from the bottom of a Vec, and reserved
grow "down" from the top of the same Vec, because we know they
can't meet in the middle. But there is a better way.

This change combines free and reserved into unused, cutting the
memory in half, and also lays them out like this:

------------------------------------------
| freelist | reserved | <room to expand> |
------------------------------------------
           ^          ^
  freelist_cursor   reserved_cursor

When reserve_entity wants to move something from the freelist
to the reserved list, it simply atomically decrements freelist_cursor,
the boundary between the two, without moving any array entries.
This avoids the previous atomic operation to update reserved.

reserved_cursor no longer needs to be atomic because it is only
modified by &mut methods.

This approach does make alloc() and free() slightly more complicated.
alloc() has to update reserved_cursor to match freelist_cursor
(just a single store). This works if you can't have unflushed data
when you alloc, which is my understanding.

If free tries to push an item and there are reserved values, it
needs to move the first reserved value to the end of the reserved
range to make room for the larger freelist. I don't know if this can
happen -- is it actually legal to free into an unflushed Entities?
If not, then we don't need some of this code.

This change also has a few other tweaks:

• When grow() copies values into the freelist, put them "before"
  the old freelist values (if any), so they get popped last. This way
  we are more likely to hand out lower IDs, only allocating higher IDs
  when we really need them.

• Added some unit tests.

I wasn't completely clear on the reserved_len (etc.) code and its
requirements (what is legal while iterating with those?), so I took my
best guess how to update it.

[cart]

I do love optimizations :)

Given that this touches hecs code, it might also be a good idea to submit a PR to the hecs repo so they can get these optimizations too. @Ralith is this something you're interested in? It would be nice to get your feedback on this too given that you wrote entities.rs.

[mjhostet]

Another idea: if it's not legal to free() while unflushed stuff exists (unclear to me), the pending atomic could be deleted, as well as the free_cursor CAS. Instead, just use simpler code that unconditionally decrements free_cursor, and the more negative it goes, the more pending values there are.

[mjhostet]

Another thing that becomes possible by not moving freelist values around is that it's easy to add a reserve_entities method that returns N entities at once. It would return an iterator that contains both a slice pointing into the Entities freelist, and a numeric range of "pending" IDs consumed if the freelist ran out. Combined with removing the pending atomic by letting free_cursor (now AtomicI64) go negative, that means that it would only take a single atomic subtract to reserve any number of IDs, and allocate no memory.

[Ralith]

These changes (subject to detailed review) would be very welcome in hecs!

is it actually legal to free into an unflushed Entities?

I'm not certain what you mean by "into" here. free is not currently supported on reserved, unflushed entities, but may in principle be called prior to flush on real entities. At least in upstream hecs, we only ever call free immediately after flush (in World::despawn), so I'd be perfectly happy to strengthen that requirement to "must only be called with no pending (i.e. unflushed) entities" if it's useful.

[mjhostet]

Thanks @Ralith, that's the strengthened requirement I was interested in. I'll put together a PR for hecs and we can talk more about it there.

[mjhostet]

I will submit something upstream, so withdrawing this PR.

[cart]

Cool cool. If/when this gets merged there, feel free to open up another pr here.

[mjhostet]

Upstream PR is Ralith/hecs#86

[mjhostet]

@cart @Ralith please let me know if there is a specific ECS benchmark you'd like me to make faster.

[Ralith]

The first thing that comes to my mind perfwise is the inlining improvements that were made in bevy which haven't yet been upstreamed. Help getting that synced back up would be great!

[cart]

Yeah I think the inlining and "Entity as query" improvements are clear / relatively easy things to upstream. Sorry @Ralith for not driving those upstream myself yet. @mjhostet you are definitely welcome to do that if you want.