Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance: Skip work in Update Kinematic Bodies if object has not moved since last update. #847

Closed
MaxCWhitehead opened this issue Jul 27, 2023 · 4 comments

Comments

@MaxCWhitehead
Copy link
Collaborator

Description

I setup a map in which I am only player (no AI), and removed critters such that nothing is actively moving.

It seems items and other kinematics are going down paths for shoving out of objects, and checking if should fall through ground running collision detection. Some bodies do check falling path and take 100ms, others seem to do shove -> check falling for 50ms each, cumulative cost seems to be 100ms consistently per object.

The 50ms cost of shove out of objects is just for collision detection, is higher if actually shoving out.

I am interested in exploring if we can "sleep" items that are not moving or interacted with. Basically if gravity is the only force acted on them, we know they should end up in the same spot as last frame, and can skip collision tests with static objects such as tilemap.

Probably should proceed with care, as these types of sleeping optimizations are easy to do wrong / get bugs, but I believe we should be able to shave something close to 1.5ms off of each game update. Possibly more in a map with more items.

Example of update loop with no critters, one player not moving, and items laying around map:

image

I expect this should have an impact on #846

@zicklag
Copy link
Member

zicklag commented Jul 28, 2023

I have been worried about the inefficiency of our "shove out of things" loop. I didn't want to pre-maturely optimize, but it felt inefficient.

I wonder if we could take better advantage of things built-into rapier, like maybe the broad phase for avoiding searching for collisions with objects not next to the object of interest.

Sleeping does sound like a good idea, too, as long as we can come up with a decent algorithm for how to wake them up.

@MaxCWhitehead
Copy link
Collaborator Author

I started optimizing that function - wrote something taking expanded rect (that we currently use corners to sample tiles with), and then sampled tiles once based on that full rect, vs doing it 4 times for each corner. Other goal was removing iterative method. Because tiles are same size / on a uniform grid, can determine overlapping cells and do collision pretty efficiently outside rapier, a 'Uniform Grid' actually is a form of broadphase in itself (just food for thought haha). Tho in this case it is specific to tiles and not a general purpose grid. Exploring options using rapier's query functions is probably a good idea. Their stuff will be less likely to have bugs, and I don't think we have enough objects to get too worried about broad-phase perf. We still have to handle resolution ourselves as kinematic, but a list of penetration info per-tile from rapier is something we should be able to process for push-out.

Didn't quite get the non-iterative resolution part right, and felt I was over-complicating things so switched gears. I started realizing that most of our costs probably aren't here, it seems like when player moves we do sweeps in rapier, so it in theory shouldn't end up inside tiles often - but I'm sure we still can. (it should be possible to avoid for normal movement entirely if we try hard enough though I think).

But we would still need a push out steps if we have teleports, objects that don't do swept movement, maybe map is edited and new tile appears on top of something, etc. So I think goal there would be reduce the occurrences of push outs, before optimizing push out itself. Then we can address that if necessary. The fact that it is iterative is a risk though, as in bad cases, can spike. There is definitely a way to implement this in non-iterative method though. I have a lot of thoughts on how we can improve perf of push-out if ever want to discuss in more detail.

Anywho, I will think about sleeping/waking system that is not going to make moving kinematic objects full of pitfalls / risk of bugs and see if get anywhere.

@MaxCWhitehead
Copy link
Collaborator Author

Another note: I haven't looked into how we move other items, how critters and stuff move, not sure if we are relying on push out for movement, or if they use swept movement (a rapier query sweeping shape along path). If they do not, we could consider leaning into this vs using push out, but there will be a cost somewhere either way. The advantage to using a swept style system is that fast moving objects will not tunnel through tiles (or thin colliders), perhaps not relevant to us atm, but this is a common pattern for handling collision of moving kinematic objects.

github-merge-queue bot pushed a commit that referenced this issue Jul 28, 2023
…h if object has not moved since last update. (#848)

So the stat shown in #847 wasn't too representative of the costs as I
had removed the critters and such.

This change checks if kinematic has moved since last update, and only
tries shove out if it has moved. (If hasn't moved, either isn't
colliding and doesn't need shove out, or is stuck and another shove out
will yield same results and remain stuck).

This removed about 1ms from update_kinematic_bodies.
<img width="881" alt="image"
src="https://github.com/fishfolk/jumpy/assets/35712032/6cd42b75-f9e2-4eb8-b166-97043953a4f5">


Would like to leave #847 open, I am aiming to also skip the fallthrough
collision check if hasn't moved, however there is additional complexity
here related to fall_through bool, and I don't want to break stuff so
that will come later. If we can skip that - should be able to get this
function to almost only spend time on moving objects and save even more.
@MaxCWhitehead
Copy link
Collaborator Author

@zicklag we're pretty fast now, while there is def stuff we can do here, I'm not sure it needs to be a priority as may be a lot of work for smaller gains. Can always revisit this issue tho / reopen, as there is valuable discussion. Feel free to re-open if you want it to remain easier to find.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants