depend on serde_derive directly for compile times

[jakobhellermann]

Objective

• by depending on serde_derive separately from serde instead of enabling the derive feature, we potentially allow compiling serde and serde_derive in parallel
  • this only works if everyone does it (but bevy is the only one after depend on serde_derive separately for more compilation parallelism ron-rs/ron#441 and remove serde_derive bitshifter/glam-rs#385)

From my quick testing, this saved 1 second on clean compile (29s -> 28s) but this is very dependant on a lot of factors, so it can vary between not mattering at all and saving multiple seconds on a slower machine in a different crate graph).

The current compilation bottleneck is
syn -> serde -> hashbrown -> indexmap -> petgraph -> bevy_utils -> bevy_reflect -> bevy_ecs.

With this PR it is
syn -> bevy_reflect_derive -> bevy_reflect -> bevy_ecs or
syn -> serde_derive -> bevy_math -> bevy_reflect, depending on which is faster

Solution

split

serde = { version = "1", feature = ["derive"]  }

into

serde = { version = "1"  }
serde_derive = { version = "1"  }

If serde was previously optional under the serde feature, add

[features]
serde = ["dep:serde", "dep:serde_derive"]

to make it work as before.

Replace

use serde::Serialize;

with

use serde_derive::Serialize;
// use serde::ser::Serialize; also this if the trait is mentioned directly

Do the same for bytemuck derive because why not, it doesn't hurt even though it's not in the critical chain.

Inspired by rust-lang/rust@fdbc3c2

Changelog

• depend on serde_derive directly. This has the potential to improve compile times

[mockersf]

Suggested change

	serialize = ["serde"]
	serde = ["dep:serde", "dep:serde_derive"]
	serialize = ["dep:serde", "dep:serde_derive"]

wouldn't it work to use the same feature instead of adding a new one?

[jakobhellermann]

I wanted to keep compatibility with users of bevy_input = { version = "0.10", features = ["serde"] }. This previously worked as every optional crate has an implicit feature.

But yeah, we shouldn't have two features for the same thing. Is there any reason for not only having the serde feature everywhere? That way we have just one thing that works all the time. I think in the past cargo couldn't have a feature of an optional crate also enable other features, but as far as I can tell this isn't a problem anymore with serde = ["dep:serde", "dep:serde_derive", "glam/serde"].

[mockersf]

I think the idea of aserialize feature was to not be explicit over serde dependency in case we want to replace it someday

[jakobhellermann]

for context (this is a release build). ron doesn't even use serde_derive but it has to wait 9.6 seconds anyways because it needs serde and serde wants to reexport serde_derive when the derive feature is active.

[james7132]

LGTM on the changes and the motivation. I'll test the change in compile time myself later today/this week.

[cart]

Some data points from my machine (linux, intel i7 7700k, 16gb ram, reasonable ssd from 5 years ago):

• cargo build this PR: 1min 12 sec, 1 min 10 sec
• cargo build commit before this pr: 1 min 10 sec, 1 min 10 sec

Seems like a no-op on my machine. I think the question is "is committing to adopting this slightly non-standard / more boilerplatey approach and holding this line in the future worth it?". I like the idea of optimizing our build graph, but the cost-benefit of this particular case is pretty uncertain / hard to nail down.

[cart]

for context (this is a release build). ron doesn't even use serde_derive but it has to wait 9.6 seconds anyways because it needs serde and serde wants to reexport serde_derive when the derive feature is active.

Ahh missed the text here. Thats definitely a bigger win.

[cart]

Actually hold on: pretty sure this means user app code and 3rd party plugins/crates will also need to use this pattern. And if they don't then we're back where we started? I'm more ok with eating this complexity internally, but I don't think foisting it on 3rd party plugins is reasonable. I highly doubt people will know or care, and I definitely don't want to campaign for alignment here / make this something people need to think about.

So if we assume that most user app code will have at least one "normal" serde derive sourced from the serde crate, is there really a point to this?

[jakobhellermann]

Actually hold on: pretty sure this means user app code and 3rd party plugins/crates will also need to use this pattern. And if they don't then we're back where we started? I'm more ok with eating this complexity internally, but I don't think foisting it on 3rd party plugins is reasonable. I highly doubt people will know or care, and I definitely don't want to campaign for alignment here / make this something people need to think about.

Yes, as soon as anyone depends on serde with the derive feature these changes in bevy don't matter. I think there probably are a good amount of games that don't use serde so maybe this is still worth it, but if you don't think this is worth it feel free to close this PR, I don't feel too strongly about it.

[jakobhellermann]

Closing this PR as any potential gains are very sensitive to other user code and other factors, so it's not a clear win for the complexity it introduces