`#[derive(Reflect)]` generates excessive amounts of code

[nnethercote]

I recently added a new -Zmacro-stats option to the Rust compiler, which prints measurements on the size of code generated by macros. I soon heard that Bevy's #[derive(Reflect)] is generating large amounts of code.

For example, consider the bevy_ui crate. It is around 16,000 lines of code, and 563,000 bytes of text. Here are the top five entries in the -Zmacro-stats output:

MACRO EXPANSION STATS: bevy_ui
Macro Name                         Uses      Lines  Avg Lines      Bytes  Avg Bytes
-----------------------------------------------------------------------------------
#[derive(Reflect)]                   73     27_407      375.4  1_544_696   21_160.2
#[derive(QueryData)]                  2      1_695      847.5     99_092   49_546.0
#[derive(Component)]                 47      1_368       29.1     70_043    1_490.3
#[derive(Debug)]                     88        930       10.6     38_349      435.8
#[derive(PartialEq)]                 76        792       10.4     34_317      451.5

All the #[derive(Reflect)] invocations in bevy_ui produce more than 1.5MB of source code, an average of 21KB per invocation. That's about 3x the size of the original code, and more than 15x the size of the next biggest derive.

The generated code for a simple example can be seen here. It is very verbose and contains multiple redundancies.

All this code has to be processed by the compiler: parsed, type-checked, borrow-checked, optimized, and codegen'd. Reducing the size of the generated code could improve compile times for Bevy projects significantly.

[mgi388]

@nnethercote is it possible to add any inline use in functions to replace all the ::core::option::Option::Some qualifications?

[nnethercote]

@mgi388: Because it's all encapsulated within a top-level const _: () = { ... } I think that should be possible. There is currently a nested const _ that has use alloc::string::ToString; within it. (Ironically, I think that use is unnecessary.)

A use bevy::reflect::* would avoid the need for many of the qualifiers. Also some ::core imports would also help. I'm not sure how much of an effect on compile time that would have -- less parsing, more name resolution, everything else no really affected, so it might be a wash. If nothing else, it would make the cargo expand output much more readable. I think it's definitely worth trying.

[bjorn3]

Ironically, I think that use is unnecessary.

It probably is necessary for #![no_std] builds.

[nnethercote]

I think it's definitely worth trying.

On second thought, I think the full paths are necessary. Imagine you have a type called Any and derive Reflect on it. You'll end up with generated code like this:

    impl ::bevy::reflect::FromReflect for Any
    where
        Any: ::core::any::Any + ::core::marker::Send + ::core::marker::Sync,
    ...

Written like that, there is no conflict between the local Any and the Any from core. But if it becomes something like this:

    use ::core::any::Any;
    impl ::bevy::reflect::FromReflect for Any
    where
        Any: Any + ::core::marker::Send + ::core::marker::Sync,
    ...

then you have two conflicting meanings of Any. (There may also be a local Any type that is not part of the derive.) I can't see a way around this.

[nnethercote]

It probably is necessary for #![no_std] builds.

How so?

[bjorn3]

In no_std builds, ToString is not part of the prelude. But I now see that it is referenced using bevy::reflect::__macro_exports::alloc_utils::ToString::to_string instead.

[nnethercote]

For example, consider the bevy_ui crate. It is around 16,000 lines of code, and 563,000 bytes of text. Here are the top five entries in the -Zmacro-stats output:

MACRO EXPANSION STATS: bevy_ui
Macro Name                         Uses      Lines  Avg Lines      Bytes  Avg Bytes
-----------------------------------------------------------------------------------
#[derive(Reflect)]                   73     27_407      375.4  1_544_696   21_160.2
#[derive(QueryData)]                  2      1_695      847.5     99_092   49_546.0
#[derive(Component)]                 47      1_368       29.1     70_043    1_490.3
#[derive(Debug)]                     88        930       10.6     38_349      435.8
#[derive(PartialEq)]                 76        792       10.4     34_317      451.5

That was with reflect_documentation enabled. When I add reflect_functions as well it becomes this:

MACRO EXPANSION STATS: bevy_ui
Macro Name                         Uses      Lines  Avg Lines      Bytes  Avg Bytes
-----------------------------------------------------------------------------------
#[derive(Reflect)]                   73     34_650      474.7  1_880_486   25_760.1
#[derive(QueryData)]                  2      1_695      847.5     99_092   49_546.0
#[derive(Component)]                 47      1_368       29.1     70_043    1_490.3
#[derive(Debug)]                     88        930       10.6     38_349      435.8
#[derive(PartialEq)]                 76        792       10.4     34_317      451.5

[notmd]

I think we can remove some trait bound for primitive type like u8, u16, u32... and assuming it already reflected

impl bevy::reflect::GetTypeRegistration for EnemySpawnerIsolated
    where
        EnemySpawnerIsolated: ::core::any::Any + ::core::marker::Send + ::core::marker::Sync,
        Annulus: bevy::reflect::FromReflect
            + bevy::reflect::TypePath
            + bevy::reflect::MaybeTyped
            + bevy::reflect::__macro_exports::RegisterForReflection,
        u32: bevy::reflect::FromReflect
            + bevy::reflect::TypePath
            + bevy::reflect::MaybeTyped
            + bevy::reflect::__macro_exports::RegisterForReflection,