Refactor into Modular Plugins

[bushrat011899]

Objective

For a long time now, there's been a desire to avoid the requirement for Reflection in order to rollback components and resources. Since #66 has been merged, we now have the option of adding completely custom rollback behaviour either as an end user, or within bevy_ggrs.

Solution

I have created a selection of plugins, which (with some helper data structures) demonstrate how we could add completely custom rollback behaviour. Because these plugins don't overlap on any mutable parameters (with some reasonable exceptions), their systems can run entirely in parallel, as scheduled by Bevy. This has the potential to provide a massive performance improvement, even without smarter implementations. For example, the Bevy smart points (Res, ResMut, Mut, Ref, etc.) all contain a Ticks item, which records when items were last added/changed. It would be pretty trivial to store that information along side the snapshot, avoiding rollback entirely for unchanged items, and thus avoiding noisy change detection (Changed, Added, etc.) even without bypass_change_detection.

Usage

When setting up an app, end users can either use extension methods provided by GgrsApp:

app.rollback_resource_with_clone::<FrameCount>()
    .rollback_component_with_clone::<Transform>()
    .rollback_component_with_clone::<Velocity>();

Or by adding the underlying plugins directly:

app.add_plugins((
    ResourceSnapshotPlugin::<CloneStrategy<FrameCount>>::default(),
    ComponentSnapshotPlugin::<CloneStrategy<Transform>>::default(),
    ComponentSnapshotPlugin::<CloneStrategy<Velocity>>::default()
));

Plugins

GgrsPlugin

The main entry point for bevy_ggrs, providing a sensible default starting point.

app.add_plugins(GgrsPlugin::<MyGameConfig>::default());

EntitySnapshotPlugin

A [Plugin] which manages the rollback for Entities. This will ensure all Entities match the state of the desired frame, or can be mapped using a [RollbackEntityMap], which this [Plugin] will also manage.

This can be considered one of the core plugins, since it is responsible for ensuring the entity graph itself is ready, allowing other systems to manage resources and components.

ComponentSnapshotPlugin<S>

A [Plugin] which manages snapshots for a [Component] a provided [Strategy] S.

#[derive(Component, Clone)]
pub struct MyVectorOfData(/* ... */);

app.add_plugins(ComponentSnapshotPlugin::<CloneStrategy<MyVectorOfData>>::default());

The three built-in strategies provided are:

• CloneStrategy: Clones data and stores as the original type.
• CopyStrategy: Copies data and stores as the original type.
• ReflectStrategy: Reflects the data, clones it, and stores as Box<dyn Reflect>.

ResourceSnapshotPlugin<S>

A [Plugin] which manages snapshots for a [Resource] a provided [Strategy] S.

#[derive(Resource, Clone)]
pub struct MyVectorOfData(/* ... */);

app.add_plugins(ResourceSnapshotPlugin::<CloneStrategy<MyVectorOfData>>::default());

The three built-in strategies provided are:

• CloneStrategy: Clones data and stores as the original type.
• CopyStrategy: Copies data and stores as the original type.
• ReflectStrategy: Reflects the data, clones it, and stores as Box<dyn Reflect>.

ChecksumPlugin

A [Plugin] which creates a [Checksum] resource which can be read after the [SaveWorldSet::Snapshot] set in the [SaveWorld] schedule has been run. To add you own data to this [Checksum], create an [Entity] with a [ChecksumPart] [Component]. Every [Entity] with this [Component] will participate in the creation of a [Checksum].

// This checksum is updated every frame in SaveWorld
let checksum = world.get_resource::<Checksum>();

Note that this plugin does not store [Checksums] for older frames.

ComponentChecksumHashPlugin<C>

A [Plugin] which will track the [Component] C on Rollback Entities and ensure a [ChecksumPart] is available and updated. This can be used to generate a Checksum.

fn get_checksum_for_foo(parts: Query<&ChecksumPart, With<<ChecksumFlag<Foo>>>>) {
    let mut hasher = DefaultHasher::new();

    for part in parts.iter() {
        part.hash(&mut hasher);
    }

    let checksum: u64 = hasher.finish();
}

ComponentMapEntitiesPlugin<C>

A [Plugin] which updates the state of a post-rollback [Component] C using [MapEntities].

#[derive(Component, Clone, Copy)]
pub struct BestFriend(Entity);

impl MapEntities for BestFriend {
    /* ... */
}

app.add_plugins((
    ComponentSnapshotCopyPlugin::<BestFriend>::default(),
    ComponentMapEntitiesPlugin::<BestFriend>::default(),
));

SnapshotSetPlugin

Sets up the [LoadWorldSet] and [SaveWorldSet] sets, allowing for explicit ordering of rollback systems across plugins.

app.add_systems(LoadWorld, log_pre_mapping_parents.before(LoadWorldSet::Mapping));

ResourceChecksumHashPlugin<R>

Plugin which will track the [Resource] R and ensure a [ChecksumPart] is available and updated. This can be used to generate a Checksum.

fn get_checksum_for_bar(parts: Query<&ChecksumPart, With<<ChecksumFlag<Bar>>>>) {
    let mut hasher = DefaultHasher::new();

    for part in parts.iter() {
        part.hash(&mut hasher);
    }

    let checksum: u64 = hasher.finish();
}

ResourceMapEntitiesPlugin<R>

A [Plugin] which updates the state of a post-rollback [Resource] R using [MapEntities].

#[derive(Resource, Clone, Copy)]
pub struct Player(Entity);

impl MapEntities for Player {
    /* ... */
}

app.add_plugins((
    ResourceMapEntitiesPlugin::<Player>::default(),
    ResourceMapEntitiesPlugin::<Player>::default(),
));

Outstanding Issues

• Components and resources which require mapping don't have an easy way to gain access to the EntityMap generated during load_world. I think this should be added as a resource to the LoadWorld schedule as a simple fix. Got a map resource available now, with some bugs. Bugs are now gone!
• I'm currently storing snapshots with a fixed depth of 60. This depth is configurable, but it should be set automatically based on the session (I believe GGRS provides a "max rollback distance" parameter we could use). I'm now asking the Session for the current confirmed_frame, and anything older than that is discarded.
• I've simply added these plugins and not integrated them with GgrsPlugin or GgrsApp. I think it might be desirable to have a clean way to add rollback functionality like register_rollback_component. All plugins are accessible through GgrsApp methods.
• I believe it is possible to create a more generic RollbackResourcePlugin/RollbackComponentPlugin that can accept a Strategy, which would be an object describing how to store, load, and update a rollback item. Implemented as described.

[johanhelsing]

Thanks for working on this!

I really like the modularity of this approach. Feels elegant that you could just write your own plugin if the ones we provide aren't a good fit. Relatively easy to understand how it works.

Components and resources which require mapping don't have an easy way to gain access to the EntityMap generated during load_world. I think this should be added as a resource to the LoadWorld schedule as a simple fix.

Yes, would probably make sense to expose a (sane) entity map as a resource... Would probably need to be two steps? One for adding mappings, and one for applying them... Not sure how that would work with our hack?

I'm currently storing snapshots with a fixed depth of 60. This depth is configurable, but it should be set automatically based on the session (I believe GGRS provides a "max rollback distance" parameter we could use)

Maybe another approach is to provide access to the latest confirmed frame, and just clear frames older than that? Knowing latest confirmed frame could be really useful anyway.

[bushrat011899]

Ok so this is turning into a pretty major re-factor of bevy_ggrs, but I'm in too deep to turn around now. I've completely removed the world_snapshot.rs file and everything it contained. Its responsibilities have instead been compartmentalised into dedicated plugins with narrowed systems. Now, instead of a pair of large save_world and load_world systems that have exclusive access to the world, there are the following plugins:

• GgrsChecksumPlugin: Responsible for taking reported checksums and combining them into a single value just for the current frame.
• GgrsComponentChecksumHashPlugin<C>: Responsible for computing the checksum of one component type C, using Hash.
• GgrsComponentSnapshotClonePlugin<C>: Responsible for saving/loading the component C using Clone.
• GgrsComponentMapEntitiesPlugin<C>: Responsible for updating the entity relationships for the component C using MapEntities.
• GgrsComponentSnapshotReflectPlugin<C>: Responsible for saving/loading the component C using Reflect + FromWorld.
• GgrsEntitySnapshotPlugin: Responsible for snaphottin and de/spawning entities during a rollback, and also creates a RollbackEntityMap that can be used by other systems.
• GgrsResourceSnapshotClonePlugin<R>: Responsible for saving/loading the resource R using Clone.
• GgrsResourceMapEntitiesPlugin<R>: Responsible for updating the entity relationships for the resource R using MapEntities.
• GgrsResourceSnapshotReflectPlugin<R>: Responsible for saving/loading the component C using Reflect + FromWorld.

Using these plugins, the TypeRegistry is completely redundant and has been removed. To get back to pre-PR behaviour, the GgrsPlugin has been modified to add the following default plugins:

app.add_plugins((
    GgrsChecksumPlugin,
    GgrsResourceSnapshotClonePlugin::<Checksum>::default(),
    GgrsEntitySnapshotPlugin,
    GgrsComponentSnapshotReflectPlugin::<Parent>::default(),
    GgrsComponentMapEntitiesPlugin::<Parent>::default(),
    GgrsComponentSnapshotReflectPlugin::<Children>::default(),
    GgrsComponentMapEntitiesPlugin::<Children>::default(),
));

While this is a lot of plugins (and I've placed each into their own file which creates a lot of verbosity compared to world_snapshot.rs), each is very clear and maintainable. For example, the plugin for getting component checksums is:

impl<C> GgrsComponentChecksumHashPlugin<C>
where
    C: Component + Hash,
{
    pub fn update(
        mut commands: Commands,
        components: Query<&C, (With<Rollback>, Without<ChecksumFlag<C>>)>,
        mut checksum: Query<&mut ChecksumPart, (Without<Rollback>, With<ChecksumFlag<C>>)>,
    ) {
        let mut hasher = DefaultHasher::new();

        let Ok(mut checksum) = checksum.get_single_mut() else {
            commands.spawn((ChecksumPart::default(), ChecksumFlag::<C>::default()));

            return;
        };

        for component in components.iter() {
            component.hash(&mut hasher);
        }

        *checksum = ChecksumPart(hasher.finish());
    }
}

impl<C> Plugin for GgrsComponentChecksumHashPlugin<C>
where
    C: Component + Hash,
{
    fn build(&self, app: &mut App) {
        app.add_systems(SaveWorld, Self::update);
    }
}

Because of how they're segregated along Component and Resource types, almost all of them can run in parallel, with the exception of some obvious dependency (e.g., can't rollback Parent until all the entities have been de/spawned, can't map until the Parent is rolled back, etc.). I still have to add some system sets to organise these dependencies, and there's currently a bug with commands not being flushed at the right time for entity mapping, but the box_game sync_test runs with all these changes, and so does the P2P example.

[johanhelsing]

Love it!

Using these plugins, the TypeRegistry is completely redundant and has been removed.

I wonder if this affects performance?

To get back to pre-PR behaviour, the GgrsPlugin has been modified to add the following default plugin

This seems super-clean to me :)

[bushrat011899]

Love it!

Thanks!

Using these plugins, the TypeRegistry is completely redundant and has been removed.

I wonder if this affects performance?

Honestly unsure. I think it would help with compiler optimisations, since the type information will be held onto all the way til it's taken by Bevy itself. What I am confident in saying is this framework would allow for the creation of the optimal solution. For example, it may be much better to create a custom plugin for adding/removing Parent and Children using the built-in Bevy commands. With this new framework, that is entirely possible, and can be done by an end user at their discretion.

To get back to pre-PR behaviour, the GgrsPlugin has been modified to add the following default plugin

This seems super-clean to me :)

Again, thanks!

[bushrat011899]

I'm marking this PR as ready for review because at this point I'd really appreciate some more specific feedback on possible changes. Currently, I believe this PR "just works", and doesn't even require any updates to existing games, since it mostly reuses existing API. A key exception is rollback resources/components added with app.register_rollback_component(...) will no longer perform entity mapping by default. I don't know how common that kind of problem would be though, and can be easily worked around by adding app.add_plugins(GgrsComponentMapEntitiesPlugin::<...>::default()).

I've made lots of very opinionated changes, but I'm completely open to revisiting any of my choices if they're a poor fit for the project.

[johanhelsing]

I like the general direction and API changes.

I really wanted to finish reviewing this today, but I noticed I got a bit tired half-way through and my comments were getting sloppier.

Not sure when I will have time to continue, so posting what I've got so far, even though there most likely are some stupid questions in there :P

What I've read so far is great, though :) Also love that you documented things fairly well.

[johanhelsing]

Currently, I believe this PR "just works", and doesn't even require any updates to existing games, since it mostly reuses existing API. A key exception is rollback resources/components added with app.register_rollback_component(...) will no longer perform entity mapping by default.

IMO it would be better to break backwards source-compatibility and just make what we think is the best API going forwards

[bushrat011899]

IMO it would be better to break backwards source-compatibility and just make what we think is the best API going forwards

I do agree with this sentiment, especially considering Bevy itself is still breaking its API at a somewhat regular pace (not a complaint, there's amazing progress being made!). I would like to have a crack at making a little plugin builder to facilitate the syntax we've discussed previously:

app.add_plugins((
    Rollback::for_resource<Foo>().with_clone().with_mapping(),
    Rollback::for_component<Bar>().with_copy(),
));

[gschup]

You hint towards the documentation of this plugin, but I cannot find any helpful text there that would further educate the user how to deal with this.

In addition to extra documentation, we probably need a parented example (e.g. cube with smaller child cube on top) that showcases usage of this.

[bushrat011899]

I agree, I've gone over all the plugins and added some extra documentation and at least 1 example demonstrating how to use the plugin. As a benefit, I've ensured every example properly compiles as a doc-test, so this has also increased test coverage.

As for the example, I completely agree. I think it would be good to maybe work on a stress-test example, as a benchmark. Since, as far as I can tell, there currently isn't really a benchmark in this crate for testing performance regression against.

[bushrat011899]

To spur discussion, I've started profiling this PR versus the current main branch, just to see what (if any) performance changes this would introduce. Running the current sync test example on my laptop, this PR increases running time for the run_ggrs_schedules system by approximately 1ms (from 2.3ms to 3.3ms). I suspect almost all of that change is the additional overhead in running many smaller systems instead of a handful of large ones. I will look into creating a proper benchmark game with far more entities, components, etc. to compare against.

My hypothesis is that for a more realistic application, the added overhead will be outweighed by the ability to run systems in parallel. Regardless of whether that theory holds or not, I will also be spending the next couple of days seeing if I can tune the performance.

[bushrat011899]

Once I got home I did some more testing, this appears to have a massive performance boost for large quantities of rollback components like I suspected. On my laptop I ran the following:

cargo run --example box_game_synctest --release --features bevy/trace_chrome -- --num-players 1000 --check-distance 5

• On main this takes an average of 1.2ms to complete the LoadWorld schedule, and 2.7ms to complete the SaveWorld.
• On this PR, it takes 3.2ms for LoadWorld and only 0.8ms for SaveWorld, for 10,000 players!

On the main branch, I can't even run above 1,000 players, it just crashes on startup. If I instead run this PR with "only" 1,000 players, loading and saving both drop down to 0.26ms. Failure to run doesn't occur until above 15,000 players. Tomorrow I'm going to try producing some charts to visualise the scaling difference. Based on this rudimentary testing tho, it appears my hunch was correct in that the overhead of additional save/load systems is totally negligible once a significant number of rollback entities and data is at play.

[gschup]

On the main branch, I can't even run above 1,000 players, it just crashes on startup. If I instead run this PR with "only" 1,000 players, loading and saving both drop down to 0.26ms. Failure to run doesn't occur until above 15,000 players. [...]

What exactly do you mean by player? Cubes per player? In any case, very cool and promising results :)

[bushrat011899]

What exactly do you mean by player? Cubes per player? In any case, very cool and promising results :)

Leaving the box game test unchanged and setting the command line numplayers option. So strictly speaking, 10,000 synctest players. I would fully expect such a test to be worthless for actual networked play, as I don't think it's be feasible to ever have 10,000 P2P sessions active (60fps, minimum of 1 byte per input packet, 10,000 packets RX and TX, totals 120KBps on its own, completely ignoring all the other overhead involved)

[bushrat011899]

Performance Testing

Equipment

• AMD Ryzen R9 5950X limited to 65W TDP and 8 cores
• 64GB of RAM
• Windows 10

Procedure

• Ran cargo run --example box_game_synctest --release --features bevy/trace_chrome -- --check-distance 5 --num-players {X} for various X values.
• Increased X until game no longer plays (crash on launch, immediate freeze, etc.)
• Used Perfetto to get average run time for PreUpdate, LoadWorld, and SaveWorld schedules.
• Collated results in Excel.

Results

Note that the vertical and horizontal axes are both logarithmic (expecting a doubling of entities to double runtime, therefore should be linear on log-log). Additionally, the vertical axis is execution time in seconds, while the horizontal axis is the num-players parameter

Interpretation

This PR improves performance for the box game sync-test example where there are more than 10 players, by an order of magnitude. Since 10 rollback entities is (in my opinion) an expected number of entities to include in a typical multiplayer game, it is reasonable to state that this PR improves performance in all expected cases.

[johanhelsing]

I had a go at making a stress test example #78. Can be cherry-picked on top of this PR.

An interesting finding is that I seem to be dropping a lot of frames when spawning a bunch of new particles, but then it runs smoothly afterwards.

I also made branch to run on main for comparison: https://github.com/gschup/bevy_ggrs/compare/main...johanhelsing:bevy_ggrs:particles-stress-test-old?expand=1

[johanhelsing]

This is already a huge improvement both in terms of extensibility and performance.

As far as I'm concerned, this is good to go :)

We can always pursue further performance improvements in other PRs.

[bushrat011899]

Ok I'm done tinkering with this PR now. I made one last change where I've created a Strategy trait, allowing for completely generic ComponentSnapshot and ResourceSnapshot plugins:

app.add_plugins((
    ComponentSnapshotPlugin::<ReflectStrategy<Parent>>::default(),
    ComponentSnapshotPlugin::<CloneStrategy<Foo>>::default(),
    ResourceSnapshotPlugin::<CopyStrategy<Bar>>::default(),
));

A Strategy just describes how to load/store/update a snapshot:

/// A [`Strategy`] based on [`Clone`]
pub struct CloneStrategy<T: Clone>(PhantomData<T>);

impl<T: Clone> Strategy for CloneStrategy<T> {
    type Target = T;

    type Stored = T;

    #[inline(always)]
    fn store(target: &Self::Target) -> Self::Stored {
        target.clone()
    }

    #[inline(always)]
    fn load(stored: &Self::Stored) -> Self::Target {
        stored.clone()
    }
}

This changes nothing about the behaviour of this PR so none of the reviews up until this point should be affected.

[bushrat011899]

Whoops! I tidied it to death, I'll fix that import issue once I'm back home!

[gschup]

Really nice idea with the Strategy!

[gschup]

After previous reviewers did such a great job, it's hard to find any glaring issues! This really feels like a giant leap forward from the ugly chunk of code that was world_snapshot.rs. Thank you so much for the work!

[gschup]

this reads much cleaner now, nice!

[bushrat011899]

After previous reviewers did such a great job, it's hard to find any glaring issues! This really feels like a giant leap forward from the ugly chunk of code that was world_snapshot.rs.

Thank you very much for saying that, it really means a lot!

Thank you so much for the work!

No trouble at all, it was my pleasure! And thanks to yourself, @johanhelsing, and @nezuo for the help along the way!