Fix child_builder and Parent / Children management

[payload]

I was looking into the tests for Parent / Children management and noted it was difficult to figure out the intended properties of push_children / insert_children, since the two existing tests were a collection of multiple assertions over some assemblages of commands.

I intend to split the tests up into a granularity to check roughly one property at a time.
The first commit contains skipped test cases for the things which do not work as I would assume. Look for #[cfg(skip)]. The other test cases should at least cover what the previous test was covering.
I intend to fix the implementation of PushChildren / InsertChildren to not panic, keep Children unique, handle PreviousParent like it is probably intended.

The hierarchy_maintenance_system is interesting too in that regard to uphold the intended properties of Parent / Children relationship, but more like fixing up things. Functions like push_children / insert_children should not depend on that system to fix up things for them.

[alec-deason]

I like the idea of having some tests here. I'm in the middle of a change to make Children unique by hashing the children. I'm curious how you intend to approach that problem.

[alec-deason]

I think the cost of this might become significant if the number of children is large and the game is making frequent hierarchy changes. That's why in #1079 I chose to make the set persistent. It makes Children more complex internally but avoids this full iteration of the list on every operation.

[payload]

removed that by now.

[payload]

Have a look again. I now "only" check for contain in the smallvec itself, though for each added child.

I refrain from maintaining a hash map or hash set in the children component itself, since I still need to maintain order with a vec, which defeats the purpose on insert and removal and imposes more allocations for many components with few children.

I refrain from creating an ad-hoc hash set to "speed up" contains checks per operation, since its a rather delicate perf optimization which needs to be benchmarked to do it properly. I could imagine for use cases with many children and many insert/removal operations, an order-independent approach is more suitable. Like going pure HashSet in that case.

[alec-deason]

Here and on line 75 you're scanning through the existing children once for every new child. I think that's worse than the previous version where you built a HashSet. That required a scan through the existing children once per command application. Though this version might actually be faster in the case where there's only a single child being added but I'd expect it to be much slower if there are multiple new children.

[payload]

I added a work in progress commit for some benchmarks. I removed an unmaintained bench and made bencher work again. Commit contains first numbers. I add an ad-hoc HashSet for more reasonable contains() checks to see what is going on. With each change I add the benchmark results to the corresponding work in progess commit to keep track. Is there a better way to keep track and collaborate?

I checked your branch and cherry-picked the benchmark:

running 3 tests
test push_children_1_parent_to_many_children ... bench: 246,145 ns/iter (+/- 36,260)
test push_children_many_parents_with_1_child ... bench: 317,596 ns/iter (+/- 47,033)
test push_children_many_parents_with_8_children ... bench: 2,226,745 ns/iter (+/- 308,667)

Compared to latest commit on my branch:

running 3 tests
test push_children_1_parent_to_many_children ... bench: 1,866,480 ns/iter (+/- 87,740)
test push_children_many_parents_with_1_child ... bench: 368,759 ns/iter (+/- 14,810)
test push_children_many_parents_with_8_children ... bench: 2,505,811 ns/iter (+/- 98,279)

There are still some correctness issues I believe but your approach is faster and it seams to be even for the single child usecase

[payload]

closed in favor of #1079