Dynamic Systems and Components

[zicklag]

Related to: #32
Resolves: #142

Note: How this is related to #32 ( StableTypeId discussion ) this is related to #32 because it changes the TypeId struct used to identify components, but it does not directly address the problem of dynamic loading of Rust plugins. See #32 (comment).

This PR will attempt to establish the ability to create systems and components that have been determined at runtime instead of compile time. I'm going to try to implement this in one PR because I won't be sure about the sufficiency of the design until I actually get a working example of a dynamically registered system and component ( which I will include with this PR ).

Note: If we want to merge pieces of this PR one at a time, I am perfectly fine with that. I am making sure that each step is cleanly separated into it's own commit and can easily be ported to an individual PR.

I'm going to try to attack this problem one step at a time:

Steps

Non-Rust Component Ids

Status: Completed in commit: "Implement Custom Component Ids"

Currently bevy_hecs uses std::any::TypeId to uniquely identify the component IDs, but this does not allow us to define components that have a non-Rust origin. The first commit in the PR has migrated all of the internals of the bevy ECS to use a new ComponentId instead that is defined as:

/// Uniquely identifies a type of component. This is conceptually similar to
/// Rust's [`TypeId`], but allows for external type IDs to be defined.
#[derive(Eq, PartialEq, Hash, Debug, Clone, Copy)]
pub enum ComponentId {
    /// A Rust-native [`TypeId`]
    RustTypeId(TypeId),
    /// An arbitrary ID that allows you to identify types defined outside of
    /// this Rust compilation
    ExternalId(u64),
}

Establish Dynamic Queries

Status: Completed in commit: "Implement Dynamic Systems"

This adds a new state type parameter to the Fetch trait. This allows compile-time constructed queries like to be constructed with State = () and runtime constructed queries with the state parameter set to a DynamicComponentQuery.

Establish Dynamic Component Insertion

Status: Completed in commit: "Add Dynamic Component Support"

This adds a new implementation of the Bundle trait, RuntimeBundle which can be used to insert untyped components at runtime by providing a buffer of bytes and the required type information.

Create Examples

Status: Completed in respective commits

We also have new examples for dynamic_systems and dynamic_components.

Remaining Work

The biggest thing necessary at this point is a solid review. There's a lot of code changed, but thankfully not a ton of new logic. The bulk of the changes are repetitive changes required to add the necessary type parameters and such for the new Fetch `State type parameter.

Otherwise, there are a couple of todo!()'s in bevy_scene because I don't know enough about the Bevy scene architecture to integrate scenes with external components yet. I think that could reasonably be left to a separate PR, but I'm fine either way.

[zicklag]

This may need to be fixed before merging. We have to find out how we want to manage the scene registry when it comes to external types. I don't have a full understanding of how it works currently so I didn't implement anything yet

I think I'm going to prioritize figuring out the dynamic component and systems approach before I try to fix this.

[zicklag]

Just an FYI, @cart probably don't waste time reviewing this yet ( even if you get time ) as I found some stuff that might need to change a little as I keep going. I'll mark the PR as not a draft when it's ready for review.

[zicklag]

OK, this is still probably a little rough around the edges, but I think the concept is laid out enough to get some review by anybody who is able to look at this. If any community members want to comment on this it'd be good to get as much feedback as possible before Carter has a chance to look at him to save him some time if there are glaring issues. 😄

[zicklag]

Hey, CI is passing now. :)

I just ran my new benchmarks on this PR and the result is ( PR in blue, master in red ):

The frame time and the CPU cycles differences are within the noise noise range on my machine ( my laptop with a web browser running on it ), but the CPU instructions are always stable pretty much within 0.01% when there are no changes so this PR clearly represents an increase of at least 1.85% increase in CPU instructions for the benchmark games. I was hoping to avoid slowing down non-scripted games, such as the current benchmark games, at all, but it looks like the new design has a slight overhead. Not sure, how much that means though.

[zicklag]

I've now successfully demonstrated both dynamic systems and dynamic_components summing up, I think, all that is necessary to allow 3rd party implementations of scripting systems for Bevy! 🎉

The examples demonstrating both dynamic systems and dynamic components are very limited in scope and do not prove that there aren't bugs when doing queries over multiple archetypes or all kinds of other variables, but for the most part I've built on the existing Bevy logic wherever possible and there is as little new logic as possible, which should help for review and stability.

I'm going to start working on a language agnostic scripting system on top of these new features. This will stay separate from Bevy, but it will help stress test these new features once I get it going.

[cart]

Awesome work! I'm curious to hear what @Ralith would think about upstreaming these changes.

[Ralith]

I'm open to the idea, though I'd need to do a proper review. The usecase is well-justified. A few initial thoughts:

RuntimeBundle

Could this be folded into EntityBuilder?

ComponentId

Maybe a custom Hash impl that discards the enum tag could help recover some of the perf here? I'm not all that concerned about an apparently purely-on-paper overhead, though.

[zicklag]

Could this be folded into EntityBuilder?

Oh, that's perfect. I had missed the EntityBuilder. I'll update that. I really hated the name RuntimeBundle anyway. 😛

Maybe a custom Hash impl that discards the enum tag could help recover some of the perf here?

That's a good idea, I'll try that out. I'll will add that I'm pretty sure that with these changes the ECS bench iteration speed is not decreased when using static systems.

As far as the iteration speed of dynamic systems ( or, probably more accurately, stateful queries ), the last I tried the ECS bench with a dynamic system the iteration speed when from ~1µs to ~100µ compared to the non-dynamic version. I'm not sure how accurate that benchmark was, though.

At this point I figured that we can be reasonably sure that we haven't lost a, so far, noticeable amount of performance for non-scripted systems. If we notice scripted systems, or stateful queries or something are slow, we can try to optimize it later after it is actually working, being that the non-dynamic features already there don't appear slower because of the new features.

[zicklag]

Using a custom hash implementation and benchmarking just the hash with criterion yields a small performance increase:

hash rust id            time:   [7.2598 ns 7.3019 ns 7.3501 ns]                          
                        change: [-7.3700% -6.4855% -5.5781%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 8 outliers among 100 measurements (8.00%)
  4 (4.00%) high mild
  4 (4.00%) high severe

hash external id        time:   [7.2684 ns 7.2878 ns 7.3092 ns]                              
                        change: [-9.0916% -8.1682% -7.3739%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 6 outliers among 100 measurements (6.00%)
  3 (3.00%) high mild
  3 (3.00%) high severe

Does this look right for a hash implementation? I'm not sure if there is a better way to do this that avoids the match to avoid the branch while still making the hash of RustTypeId unique compared to ExternalId. In this case I just write an extra byte as either a 1 or a 0 to distinguish them from each-other. Maybe that one byte is smaller than the discriminate used in the derived hash implementation and that's why it's faster:

#[derive(Debug, Clone)]
pub enum ComponentId {
    RustTypeId(std::any::TypeId),
    ExternalId(u64),
}

impl Hash for ComponentId {
    fn hash<H: Hasher>(&self, state: &mut H) {
        match self {
            ComponentId::RustTypeId(id) => {
                id.hash(state);
                state.write_u8(0);
            }
            ComponentId::ExternalId(id) => {
                state.write_u64(*id);
                state.write_u8(1);
            }
        }
    }
}

Edit: Opened a forum topic just to get more feedback.

Also it doesn't effect the CPU instructions measurement noticeably for the asteroids and breakout benchmarks.

[zicklag]

OK, I got help on the forum that said that the hashes are allowed to collide, so we can totally get rid of any discriminant between RustTypeId and ExternalId ( which is probably exactly what you were meaning @Ralith, I just didn't know that hashes were allowed to collide, so I didn't think of that ). With that optimization we get a decent speed up:

impl Hash for ComponentId {
    fn hash<H: Hasher>(&self, state: &mut H) {
        match self {
            ComponentId::RustTypeId(id) => {
                id.hash(state);
            }
            ComponentId::ExternalId(id) => {
                state.write_u64(*id);
            }
        }
    }
}

hash rust id            time:   [5.3033 ns 5.3093 ns 5.3154 ns]                          
                        change: [-33.204% -32.628% -31.900%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 12 outliers among 100 measurements (12.00%)
  1 (1.00%) low severe
  3 (3.00%) low mild
  3 (3.00%) high mild
  5 (5.00%) high severe

hash external id        time:   [4.5068 ns 4.5136 ns 4.5208 ns]                              
                        change: [-43.353% -42.849% -42.378%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 10 outliers among 100 measurements (10.00%)
  4 (4.00%) high mild
  6 (6.00%) high severe

---

Hey that also brings down the CPU instructions a little bit in the game benchmarks. Still a little higher, but not as much as before.

[zicklag]

OK, I did the cleanup I needed to do and went over the code one more time and I think it's ready for review, now.

[alice-i-cecile]

Supporting this behavior (in particular, dynamically adding systems) has some painful downstream limitations on future optimizations: see #1205 for an example, but I would expect to see more examples in areas that touch on scheduler logic.

This is a super cool batch of changes though, and I'd love to find a compromise position! Before we merge this, can we guarantee that systems are only being added at a specific time (e.g. between frames), and maybe emit some sort of event so then any of the more advanced optimizations will know when to rebuild?

[zicklag]

Actually, this PR doesn't handle letting you add systems after starting the app, so it won't effect the scheduling performance in that respect. For instance, in the dynamic systems example, we add the dynamic system when building the app, just like we typically do in a Bevy project:

bevy/examples/ecs/dynamic_systems.rs

Line 151 in 40a70b4

.add_system(Box::new(pos_vel_system))

The primary concern of this PR was to allow dynamically adding systems without having to recompile not necessarily without having to restart. Now I could see wanting to add systems in-between frames or something like that being useful, but that would be separate from these core changes.

I'm planning on coming back to these changes and, apparently, doing a massive rebase to resolve the merge conflicts, probably sometime within the next couple months.

[zicklag]

@cart I saw you mention somewhere around here that you were working on an ECS internal rewrite that would make integrations like this scripting stuff easier. I'm maybe going to find time to finish this back up, but not immediately, and I thought maybe it would be good to wait for whatever reorganization you were doing so that I don't get in the way or have to deal with more merge conflicts later.

If that's right, and I didn't misinterpet something somewhere ( I can't find that comment I thought I saw ), could you let me know what that rewrite is done so that I can take another look at this once I get time?

[cart]

Sure thing! I should be ready for a draft pr in the next couple of days. The relevant changes:

1. Components and "types" are now decoupled. TypeIds can correlate to ComponentIds, but ComponentIds do not require TypeIds
2. New "type agnostic" datastructures: BlobVec, Table (built on top of BlobVec), SparseSet (built on top of BlobVec)
3. Stateful Queries. State can be passed into Queries. Currently this is designed for tracking archetype matches, but it could be extended to include other metadata.

[zicklag]

Awesome! That sounds like it tackles all of my pain points in implementing this!

[Defman]

I'm in the process of create a plugin system that uses bevy ecs, this is my current thoughts.

1. Client defines a system and passes a serializable version QueryAcess.
2. The host use the serializable QueryAcess to create a bevy QueryAcces and a dynamic query.
3. The host defines a DynamicSystemSettings with a workload that extracts all components and serialize them.
4. The host passes the serialized components to the client.
5. The client reconstructs a bevy query from the passed serialized components.
6. The client calls the system with the reconstructed components.

*To support mutable components would require the client to pass back mutated components and having the host to mutate the component with the new data.

*Note I'm planing on using bincode to serialize all components, therefor I belive the TypeInfo is Vec<u8> for all components.

[zicklag]

@Defman That sounds reasonable. The cool part about what I was trying to do with this PR is that it simply handles adding the ability to add dynamic systems and components, and it doesn't care how you use that ability.

The thing that sounds possibly inefficient ( to my untrained ears ) about your plan is that it sounds like you'll essentially be serializing the entire piece of the world that the plugin wants to access for every query, and I'm not sure how well that would scale. It also just depends on how performant it needs to be for your game.

Granted, I'm not sure there are any better easy options. You might be able to try to take slices of bytes for the components in your query and casting those bytes to a #[repr(C)] struct in the client, but that's not going to be necesarily possible depending on your client language.

[Defman]

@zicklag I too have some concerns with performance, but for now it's more of a proof of concept. What I really want is to know if I should implement my own Query struct, since the native one requires access to the world.

Note I have thought about using repr(C), but since I'm using wasm for the client I would have to copy the bytes and fix pointers. Not to mention the layout may differ between multiple clients and host, due to 32 vs 64bit.

[zicklag]

What I really want is to know if I should implement my own Query struct, since the native one requires access to the world.

I'm not sure yet. It sounds like a lot will change with the internal rewrite, so you might want to wait to see what that looks like once it's done. I'm waiting for that before I pick this PR back up again.

[zicklag]

@cart with the merge of #1525 the ECS should be ready for me to take another hack at this right? Nothing else I should wait for?

I'm not positive when I'll get the chance but probably soon-ish.

[cart]

Yup! Nothing else major in the pipeline that would overlap with this work.

[alice-i-cecile]

@zicklag, @BoxyUwU has been working on "relationships" to solve #1592 using #1527. It's probably worth coordinating this with that work in some fashion :) We've largely been discussing this on Discord in the #ecs dev channel. You can see the status of that project here.

[zicklag]

Thanks for the pointer @alice-i-cecile! I might be able to get to re-visiting this stuff in a week or two. I haven't checked out Bevy's new ECS updates yet so I'm going to spend some time understanding that before I do much.

Thanks for the link to the note. 👍 I'm fairly certain the exact contents of this PR are pretty much trash at this point, but I'm totally fine with that as most of my work might already be done/getting done by @BoxyUwU!

Quick summary of what I know now:

• With stateful queries in the new Bevy ECS I think that takes out a good 90% of the work that went into this PR.
• For dynamic systems, I think I literally just have to create a struct that implements IntoSystem which will provide a way to create systems without compile-time defined functions. Piece of 🍰
• Dynamic components are another story and totally depends on the new storage system, but it sounds like @BoxyUwU and company have really started to think the use-case through rather well.

I'm totally happy letting @BoxyUwU take care of the dynamic components and then adding the Dynamic systems myself, which BTW really doesn't have to come with Bevy, so maybe that doesn't even mean a Bevy PR, I can just add it to my scripting system crate.

I've got enough to work on from the FFI and language abstraction layer for the scripting system so if you guys handle the Bevy side of it that would be great!

---

As far as the notes on hackmd.io I really like the idea of dogfooding the static API on top of the dynamic API. 🎉

[zicklag]

In fact, I'm going to close this as any new effort should really be done in new PR's anyway and things have changed quite a bit. Any more discussion on this could move to #142.