trafficstars

Objective

Improve #3953

Solution

The very specific circumstances under which the render world is reset meant that the flush_as_invalid function could be replaced with one that had a noop as its init method.
This removes a double-writing issue leading to greatly increased performance.

Running the reproduction code in the linked issue, this change nearly doubles the framerate.

Jul 31 '22 11:07 TheRawMeatball

Neat. This is the most dramatic one of these I've seen.

just the reserve_and_flush span (repro from issue)

Jul 31 '22 23:07 rparrett

Wow yeah thats a big win right there. Pretty sure this is valid. I'll give this a proper think asap.

Aug 01 '22 00:08 cart

Changed again to a fully custom flush method that directly uses memset, this is getting even larger gains.

Aug 01 '22 07:08 TheRawMeatball

Currently UB, working on a fix.

Aug 01 '22 12:08 TheRawMeatball

Reverted the memset, so this works again, but others investigating why the memset version failed via UB would be appreciated.

Aug 01 '22 13:08 TheRawMeatball

UB fixed with help from @PROMETHIA-27

Aug 01 '22 13:08 TheRawMeatball

b02a1d741c02174269311fa36210e15c740ba048 (repro from issue) just the reserve_and_flush span Screen Shot 2022-08-01 at 6 41 39 AM

Aug 01 '22 13:08 rparrett

many_sprites frame time

many_spites frame time (run 2)

many_cubes sphere frame time

In all cases, yellow / this trace is this PR, red is main. Guessing we're just in the noise a bit and this is not a quiet machine at the moment. All traces were ~30 seconds though.

Aug 01 '22 14:08 rparrett

I'm excited for this PR as it seems like it fixes part of the performance issues here! I'm still worried that this isn't a complete fix though. ~~This code is still reserving entities even if they have been despawned which leads to memory leakage(increased memory usage over time) and possibly over time slow performance. I can think of quite a few scenarios where users would spawn high entity counts and despawn said entities. A shmup would be one such example where you would be spawning hundreds or even thousands of bullets which get depsawned.~~ Looking at it closer it looks like it just reserves the number of entities that currently exist. I still think performance could be an issue for me, but I think a better solution might be to have a separate world.

Aug 11 '22 12:08 StarArawn

Just checking on the general status of this PR. I noticed it was removed from the 0.8.1 release. Curious as to when we might expect this to be part of mainline. Thanks!