bevy icon indicating copy to clipboard operation
bevy copied to clipboard

Reflect auto registration

Open eugineerd opened this issue 1 year ago • 6 comments

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 reflect example and removing explicit type registration, both on native and on wasm32-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%

eugineerd avatar Sep 03 '24 15:09 eugineerd

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?

alice-i-cecile avatar Sep 03 '24 15:09 alice-i-cecile

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.

github-actions[bot] avatar Sep 03 '24 15:09 github-actions[bot]

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.

eugineerd avatar Sep 03 '24 16:09 eugineerd

~~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.

eugineerd avatar Sep 04 '24 08:09 eugineerd

IIUC, this documentation section should probably be updated: https://docs.rs/bevy_reflect/latest/bevy_reflect/#manual-registration

nixpulvis avatar Sep 07 '24 19:09 nixpulvis

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.

eugineerd avatar Dec 01 '24 15:12 eugineerd

You added a new example but didn't add metadata for it. Please update the root Cargo.toml file.

github-actions[bot] avatar Dec 08 '24 22:12 github-actions[bot]

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.

coolcatcoder avatar Jan 30 '25 02:01 coolcatcoder

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.

github-actions[bot] avatar Mar 12 '25 21:03 github-actions[bot]

it would be interesting to benchmark compilation duration with this change to see the impact

mockersf avatar Mar 18 '25 12:03 mockersf

Cutting from 0.16 while we work out some of the potential issues raised by @cart on Discord.

MrGVSV avatar Mar 18 '25 16:03 MrGVSV

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.

github-actions[bot] avatar Apr 03 '25 15:04 github-actions[bot]

Added support for platforms without inventory support (auto-register-static feature). The main caveats of the static approach are:

  1. load_type_registrations! macro must be called before constructing App or using TypeRegistry::register_derived_types.
  2. 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.
  3. Registration function names are cached in target/type_registrations. This cache might become stale if multiple projects using BEVY_REFLECT_AUTO_REGISTER_STATIC=1 are compiled in the same workspace. Due to incremental compilation the only way to rebuild this cache is to build with bevy/reflect_auto_register_static (or auto_register_static if just using bevy_reflect) feature disabled, then delete target/type_registrations and rebuild again with this feature enabled and BEVY_REFLECT_AUTO_REGISTER_STATIC=1 environment variable set. Running cargo clean before recompiling is also an option, but it is even slower to do.
  4. Increases compilation time.

All of these downsides are applicable only when using the static approach, inventory-based approach is unaffected by this.

eugineerd avatar Apr 03 '25 19:04 eugineerd

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 :)

janhohenheim avatar Jul 18 '25 00:07 janhohenheim

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.

ChristopherBiscardi avatar Aug 06 '25 01:08 ChristopherBiscardi

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.

eugineerd avatar Aug 06 '25 14:08 eugineerd

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.

cart avatar Aug 06 '25 18:08 cart