Reflect auto registration
Objective
This PR adds automatic registration of non-generic types annotated with #[derive(Reflect)].
Manually registering all reflect types can be error-prone, failing to reflect during runtime if some type is not registered. #5781 fixed this for nested types, improving ergonomics of using reflection dramatically, but top-level types still need to be registered manually. This is especially painful when prototyping and using inspector to change values of components.
// from #5781
#[derive(Reflect)]
struct Foo(Bar);
#[derive(Reflect)]
struct Bar(Baz);
#[derive(Reflect)]
struct Baz(usize);
fn main() {
// ...
app.register_type::<Foo>() // <- can still forget to add this
// ...
}
Solution
Automatically register all types that derive Reflect. This works for all non-generic types, allowing users to just add #[derive(Reflect)] to any compatible type and not think about where to register it.
fn main() {
// No need to manually call .register_type::<Foo>()
App::new()
.add_plugins(DefaultPlugins)
.add_systems(Startup, setup)
.run();
}
#[derive(Reflect)]
pub struct Foo {
a: usize,
}
fn setup(type_registry: Res<AppTypeRegistry>) {
let type_registry = type_registry.read();
assert!(type_registry.contains(TypeId::of::<Foo>()));
}
Automatic registration can be opted-out of by adding #[reflect(no_auto_register)] reflect attribute to a type:
#[derive(Reflect)]
#[reflect(no_auto_register)]
pub struct Foo {
a: usize,
}
Registration normally happens at app creation time, but TypeRegistry itself has a new register_derived_types method to register all collected types at any appropriate time:
#[derive(Reflect)]
struct Foo {
name: Option<String>,
value: i32
}
let mut type_registry = TypeRegistry::empty();
type_registry.register_derived_types();
assert!(type_registry.contains(TypeId::of::<Foo>()));
Testing
- Tested by running
reflectexample and removing explicit type registration, both on native and onwasm32-unknown-unknown. - Added a doctest to
register_derived_types
Impact on startup time is negligible: <40ms on debug wasm build and <25ms on debug native build for 1614 registered types .
Wasm binary size comparison. All sizes are in KB as reported by du on the modified reflection example.
| auto register, KiB | no auto register, KiB | abs diff, KiB | % diff | |
|---|---|---|---|---|
| wasm-release && wasm-bindgen | 27000 | 24224 | 2776 | 11.46% |
| wasm-opt -Oz | 16376 | 15340 | 1036 | 6.75% |
| wasm-opt -Os | 17728 | 16424 | 1304 | 7.94% |
| wasm-opt -Oz && gzip -c | 5620 | 5364 | 256 | 4.77% |
Considerations
While automatic type registration improves ergonomics quite a bit for projects that make heavy use of it, there are also some problems:
- Generic types can't be automatically registered. However, as long as top-level reflect types don't need generics, recursive registration can take care of the rest.
- Wasm bundle size increase is not insignificant. This is mostly due to registering various engine types that weren't registered before. Size overhead of automatic registration of types instead of manual is relatively small, as can be see from this (somewhat outdated but still relevant) table (see last 2 rows):
1000 simple structs with reflect wasm size comparison, no engine types
| auto register, KiB | no auto register, KiB | abs diff, KiB | % diff | |
|---|---|---|---|---|
| wasm-release && wasm-bindgen | 27944 | 21644 | 6300 | 22.55% |
| wasm-release && wasm-bindgen +register_type | 28084 | 27764 | 320 | 1.14% |
| wasm-opt -Oz | 15612 | 13928 | 1684 | 10.79% |
| wasm-opt -Oz +register_type | 15676 | 15580 | 96 | 0.61% |
| wasm-release && wasm-bindgen (manual vs auto) | 27944 | 27764 | 180 | 0.64% |
| wasm-opt -Oz (manual vs auto) | 15612 | 15580 | 32 | 0.2% |
Wasm bundle size increase is not insignificant
Is this compared to the equivalent manual registration? Can / should we feature flag this for authors who particularly care?
You added a new feature but didn't update the readme. Please run cargo run -p build-templated-pages -- update features to update it, and commit the file change.
Is this compared to the equivalent manual registration
As far as I understand it, wasm-init works by exporting a bunch of symbols to final wasm and calling them from outside when wasm_init() is called for the first time. I think most of the size increase is due to these exports, but I'm not sure. In the table I just built the modified registration example and compared the .wasm file sizes using du. As the number of registered types from type_registry.iter() differs (445 without, 614 with), it is not the same as manually registering types.
~~Managed to reduce wasm size considerably by removing closures from automatic type registration:~~
All sizes are in KB as reported by du on reflection example.
| wasm-release && wasm-bindgen | wasm-opt -Oz | wasm-opt -Os | wasm-opt -Oz && gzip -c | |
|---|---|---|---|---|
| +reflect_auto_register | 22560 | 14332 | 15352 | 5044 |
| default | 21716 | 13968 | 14896 | 4944 |
| diff | +3.7% | +2.5% | +3% | +2% |
~~I'm not very familiar with all the internal reflection machinery, maybe this can be optimized even further?~~
Update: Disregard that, I misunderstood how reflection works and missed registering type data.
IIUC, this documentation section should probably be updated: https://docs.rs/bevy_reflect/latest/bevy_reflect/#manual-registration
Added a basic test for no_auto_register, there is also a doctest that should cover most of auto register functionality.
but can you also add some unit tests?
What other tests do you have in mind? I'm not really sure what else should be tested here, so If you have any ideas I'd love to hear them.
You added a new example but didn't add metadata for it. Please update the root Cargo.toml file.
Inventory supports wasm. Perhaps consider using inventory for both wasm and non-wasm now. (Make sure to run __wasm_call_ctors, as is explained in Inventory's documentation.)
Edit: Apparently it may have a version requirement, so you can probably ignore me.
Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke! You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-15030
If it's expected, please add the M-Deliberate-Rendering-Change label.
If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.
it would be interesting to benchmark compilation duration with this change to see the impact
Cutting from 0.16 while we work out some of the potential issues raised by @cart on Discord.
The generated examples/README.md is out of sync with the example metadata in Cargo.toml or the example readme template. Please run cargo run -p build-templated-pages -- update examples to update it, and commit the file change.
Added support for platforms without inventory support (auto-register-static feature). The main caveats of the static approach are:
load_type_registrations!macro must be called before constructingAppor usingTypeRegistry::register_derived_types.- All of the types to be automatically registered must be declared in a separate from
load_type_registrations!crate. This is why this example uses separate lib and bin setup. - Registration function names are cached in
target/type_registrations. This cache might become stale if multiple projects usingBEVY_REFLECT_AUTO_REGISTER_STATIC=1are compiled in the same workspace. Due to incremental compilation the only way to rebuild this cache is to build withbevy/reflect_auto_register_static(orauto_register_staticif just usingbevy_reflect) feature disabled, then deletetarget/type_registrationsand rebuild again with this feature enabled andBEVY_REFLECT_AUTO_REGISTER_STATIC=1environment variable set. Running cargo clean before recompiling is also an option, but it is even slower to do. - Increases compilation time.
All of these downsides are applicable only when using the static approach, inventory-based approach is unaffected by this.
Added this to the milestone after the feature freeze after getting the okay from @alice-i-cecile. AFAIK, the only thing this needs is attention from Cart :)
This does mean we leak, as old registrations aren't removed. I think this is acceptable.
Does this mean that external users of the type registry (such as through the BRP registry schema endpoint) will receive no-longer-"valid" types? so the only way (at the moment) to ensure that (ex: Component) types are valid when fetching the endpoint would be to have a freshly booted application.
Does this mean that external users of the type registry (such as through the BRP registry schema endpoint) will receive no-longer-"valid" types?
New types will only really be registered if the type definition changed, since TypeId's seem to be stable between hotpatching applications. It should technically be possible to remove all automatically registered types from the registry on hotpatch before readding back only valid ones (since all constructors should rerun on hotpatch, but I'm not sure about it). Although we'll first need to solve the problem of data layout migration before changing existing type definitions between hotpatches becomes possible.
Does this mean that external users of the type registry (such as through the BRP registry schema endpoint) will receive no-longer-"valid" types? so the only way (at the moment) to ensure that (ex: Component) types are valid when fetching the endpoint would be to have a freshly booted application.
In a way these types could be "still valid" in that there may be instances of the old version still floating around in the code somewhere. As eugineerd said, if we're going to have one canonical version of a type in hot reloading cases, handling this will involve data layout migration. I've laid out rough plans for this in the past, but its not currently top of my list.