crystalorb
crystalorb copied to clipboard
Avoid making unnecessary copies of the displayable state
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.
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 aclear_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
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 Snapshot
s 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.