ECS: put strings only used for debug behind a feature

[mockersf]

Objective

• Many strings in bevy_ecs are created but only used for debug: system name, component name, ...
• Those strings make a significant part of the final binary and are no use in a released game

Solution

• Use strings to find ... strings in a binary
• Try to find where they come from
• Many are made from type_name::<T>() and only used in error / debug messages
• Add a new structure DebugName that holds no value if debug feature is disabled
• Replace core::any::type_name::<T>() by DebugName::type_name::<T>()

Testing

Measurements were taken without the new feature being enabled by default, to help with commands

File Size

I tried building the breakout example with cargo run --release --example breakout

debug enabled	debug disabled
81621776 B	77735728B
77.84MB	74.13MB

Compilation time

hyperfine --min-runs 15 --prepare "cargo clean && sleep 5" 'RUSTC_WRAPPER="" cargo build --release --example breakout' 'RUSTC_WRAPPER="" cargo build --release --example breakout --features debug'

breakout' 'RUSTC_WRAPPER="" cargo build --release --example breakout --features debug'
Benchmark 1: RUSTC_WRAPPER="" cargo build --release --example breakout
  Time (mean ± σ):     84.856 s ±  3.565 s    [User: 1093.817 s, System: 32.547 s]
  Range (min … max):   78.038 s … 89.214 s    15 runs

Benchmark 2: RUSTC_WRAPPER="" cargo build --release --example breakout --features debug
  Time (mean ± σ):     92.303 s ±  2.466 s    [User: 1193.443 s, System: 33.803 s]
  Range (min … max):   90.619 s … 99.684 s    15 runs

Summary
  RUSTC_WRAPPER="" cargo build --release --example breakout ran
    1.09 ± 0.05 times faster than RUSTC_WRAPPER="" cargo build --release --example breakout --features debug

[alice-i-cecile]

I don't understand your table there 🤔 Why are there two rows?

[mockersf]

I don't understand your table there 🤔 Why are there two rows?

bytes and megabytes!

[alice-i-cecile]

Hmm. I don't love the increased feature flagging, but I think that this is a reasonable optimization to make, especially in the context of games being deployed to web.

The strategy looks reasonable to me: top-level flag, trickling down, on-by-default.

I'm sure there's more places we could use this, but I'm happy with this as a start.

[mgi388]

To me the approach does look useful. I left some comments which I hope are helpful. I think I'll leave it to others to given approval though.

[mgi388]

Is it worth a comment describing the scope of debug? It reminds me of the the old if environment == dev checks in code which can become grab bags of everything.

For example in my game I have an entity_names feature:

commands.spawn((
  #[cfg(feature = "entity_names")]
  Name::new("Foo"),
));

I could have used a dev or debug feature, but I wanted to separate the actual feature in case I want to turn just this feature on for release builds.

To be clear, I think the feature debug probably makes sense compared to trying to break it down into many little features, but I just wonder if there's room to describe the scope of what a future Bevy developer should expect or be allowed to use this feature for?

[mockersf]

I thought about doing different features for things like system_name, component_name, ... but that would make the features more complex, with more tests needed, and bigger conditions when needing both.

I think a single feature debug (or maybe with a name around diagnostics? I'm open to another name) is better

[mgi388]

I personally like the name and I don't think it comes across as a grab bag. I just thought a comment here on the possible scope could be useful, but comment is non-blocking. Maybe over time it will be come clear and obvious what should and shouldn't go in this feature.

[alice-i-cecile]

I think debug_info would be much clearer and less likely to conflict as a name.

[mgi388]

What could this actually conflict with?

There is also https://doc.rust-lang.org/rustc/codegen-options/index.html#debuginfo, which could be cause for confusion, not sure though.

But yeah both sound fine to me. If I had to pick myself I'd probably go with debug, but I don't have a strong argument more than "adding _info at the end feels overly specific to how the code that it's enabling is used".

[alice-i-cecile]

I was thinking user defined dev-mode flags? I also think it's a clearer / narrower purpose that will allow us to more clearly decide whether or not something else belongs in this flag.

[mockersf]

Switched to a new DebugName struct that wraps the feature, and using DebugName::type_name::<T>() instead of core::any::type_name::<T>().

That made the diff smaller, and made it easier to use it in more places which increased a bit the size reduction, now 3.7MB!