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

Avoid making unnecessary copies of the displayable state #3

Open
ErnWong opened this issue Jun 9, 2021 · 2 comments
Open

Avoid making unnecessary copies of the displayable state #3

ErnWong opened this issue Jun 9, 2021 · 2 comments

Comments

@ErnWong
Copy link
Owner

ErnWong commented Jun 9, 2021

This repo started out as a simple state syncing implementation, and to make things easier to implement and more generalisable, the design did not focus on memory footprint and the performance penalties due to unnecessary copying of data.

For example, the client code results in having roughly 6 copies of the entire simulation state at any time! This is due to needing:

  • Two complete worlds: One without the latest snapshot and one with the latest snapshot
  • The next queued snapshot to be applied next
  • The undershot and overshot display states used for deriving the tweened display state
  • The tweened display state itself to be rendered on screen.

On discord, @TheRawMeetball suggested something like the following to be added to the World trait, in place of the DisplayState trait.

type DisplayTarget;
fn write_to_display(&self, target: &mut Self::DisplayTarget, tween: f32);

We could similarly extend this concept to allow for "blending" between the old and new worlds. Things to account for:

  • Generalisability and composability. DisplayState can currently be composed together in different ways to support different kinds of synchronization strategies. Do we still want some kind of composability?
  • It will need to support both client's and server's use case. Client uses the tweened version for rendering, and possibly either the tweened version, undershot version or overshot version of the display state for external game logic. The server might also use the undershot version of the display state to perform external game logic, or to render a diagnostic view of the world.
  • The server may need to know what display state the client observed at a particular timestamp to validate a command (e.g. hit validation with lag compensation in shooter games).

To verify the changes and explore new designs, it might be good to first setup some benchmarking.

@ErnWong
Copy link
Owner Author

ErnWong commented Jun 9, 2021

This might require more thinking than anticipated. In addition to the things to account for that are mentioned above, to tween, we need to keep a copy of the previous state. The above suggestion moves the responsibility to keep this copy from CrystalOrb to the downstream World implementation, which is not ideal:

  • Feels like this leaks some of the logic into the downstream library consumers
  • The World implementation will have no choice but to save a copy of the required states on every simulation frame even when it is only needed after the current batch of simulation frames for the current render frame. Although our current implementation does save a copy of the DisplayState after every simulation frame, it is very possible to optimise it within CrystalOrb since CrystalOrb is aware of how the simulation frames are batched together.
  • The downstream library consumers takes on the burden to ensure that their copy of the previous states are properly invalidated/cleared whenever we perform a timeskip (which the downstream libraries should not be aware of) or an apply_snapshot. We can put in something like a clear_copy function in the trait, but does it makes sense to do so?

At least, in theory, we could shrink down the number of copies of the entire world state from 6 copies down to maybe 3 copies:

  • Old world (for blending / error smoothing)
  • New world
  • Previous frame's state (for tweening)

However, the number of copies needed might grow further when we take into account #4

@ErnWong
Copy link
Owner Author

ErnWong commented Jun 10, 2021

Since this might add a fair bit of complexity / refactoring to the codebase, we should definitely set up benchmarking first before blindly optimizing things.

For example, one could make the following argument: If a game's DisplayState is so big that making multiple copies of it hurts the game's performance or memory footprint, then the corresponding Snapshots are probably too big to send over the network efficiently anyway (at least with CrystalOrb). I have no idea if this is true or not without some kind of benchmarking/profiling to measure from, but it suggests the possibility that CrystalOrb would be bottlenecked by much more than just making too many copies of DisplayState.

For now, it is probably wise to put a warning in the README that CrystalOrb may not scale well to larger games.

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

1 participant