Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Decouple serde from its derive crate #6917

Merged
merged 1 commit into from
Aug 29, 2023

Conversation

CryZe
Copy link
Contributor

@CryZe CryZe commented Aug 27, 2023

By not activating the derive feature on serde, the compilation speed can be improved by a lot. This is because serde can then compile in parallel to serde_derive, allowing it to finish compilation possibly even before serde_derive, unblocking all the crates waiting for serde to start compiling much sooner.

As it turns out the main deciding factor for how long the compile time of a project is, is primarly determined by the depth of dependencies rather than the width. In other words, a crate's compile times aren't affected by how many crates it depends on, but rather by the longest chain of dependencies that it needs to wait on. In many cases serde is part of that long chain, as it is part of a long chain if the derive feature is active:

proc-macro2 compile build script > proc-macro2 run build script > proc-macro2 > quote > syn > serde_derive > serde > serde_json (or any crate that depends on serde)

By decoupling it from serde_derive, the chain is shortened and compile times get much better.

Check this issue for a deeper elaboration:
serde-rs/serde#2584

For wasmtime I'm seeing a reduction from 24.75s to 22.45s when compiling in release mode. This is because wasmtime through gimli has a dependency on indexmap which can only start compiling when serde is finished, which you want to happen as early as possible so some of wasmtime's dependencies can start compiling.

To measure the full effect, the dependencies can't by themselves activate the derive feature. I've upstreamed a patch for fxprof-processed-profile which was the only dependency that activated it for wasmtime (not yet published to crates.io). wasmtime-cli and co. may need patches for their dependencies to see a similar improvement.

@CryZe CryZe requested review from a team as code owners August 27, 2023 20:01
@CryZe CryZe requested review from fitzgen and removed request for a team August 27, 2023 20:01
@github-actions github-actions bot added cranelift Issues related to the Cranelift code generator cranelift:meta Everything related to the meta-language. labels Aug 27, 2023
Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! One comment below.

Cargo.lock Outdated Show resolved Hide resolved
@pchickey
Copy link
Collaborator

#6921 provides the cargo vet imports for this PR, which has to be done by core contributors. Please merge with main once that PR has landed.

@fitzgen
Copy link
Member

fitzgen commented Aug 28, 2023

That PR merged, so this can be rebased now.

By not activating the `derive` feature on `serde`, the compilation speed
can be improved by a lot. This is because `serde` can then compile in
parallel to `serde_derive`, allowing it to finish compilation possibly
even before `serde_derive`, unblocking all the crates waiting for
`serde` to start compiling much sooner.

As it turns out the main deciding factor for how long the compile time of a
project is, is primarly determined by the depth of dependencies rather
than the width. In other words, a crate's compile times aren't affected
by how many crates it depends on, but rather by the longest chain of
dependencies that it needs to wait on. In many cases `serde` is part of
that long chain, as it is part of a long chain if the `derive` feature
is active:

`proc-macro2` compile build script > `proc-macro2` run build script >
`proc-macro2` > `quote` > `syn` > `serde_derive` > `serde` >
`serde_json` (or any crate that depends on serde)

By decoupling it from `serde_derive`, the chain is shortened and compile
times get much better.

Check this issue for a deeper elaboration:
serde-rs/serde#2584

For `wasmtime` I'm seeing a reduction from 24.75s to 22.45s when
compiling in `release` mode. This is because wasmtime through `gimli`
has a dependency on `indexmap` which can only start compiling when
`serde` is finished, which you want to happen as early as possible so
some of wasmtime's dependencies can start compiling.

To measure the full effect, the dependencies can't by themselves
activate the `derive` feature. I've upstreamed a patch for
`fxprof-processed-profile` which was the only dependency that activated
it for `wasmtime` (not yet published to crates.io). `wasmtime-cli` and
co. may need patches for their dependencies to see a similar
improvement.
Copy link
Member

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@fitzgen fitzgen added this pull request to the merge queue Aug 29, 2023
Merged via the queue into bytecodealliance:main with commit 9ec02f9 Aug 29, 2023
18 checks passed
@CryZe CryZe deleted the decouple-serde-derive branch September 1, 2023 19:30
eduardomourar pushed a commit to eduardomourar/wasmtime that referenced this pull request Sep 6, 2023
By not activating the `derive` feature on `serde`, the compilation speed
can be improved by a lot. This is because `serde` can then compile in
parallel to `serde_derive`, allowing it to finish compilation possibly
even before `serde_derive`, unblocking all the crates waiting for
`serde` to start compiling much sooner.

As it turns out the main deciding factor for how long the compile time of a
project is, is primarly determined by the depth of dependencies rather
than the width. In other words, a crate's compile times aren't affected
by how many crates it depends on, but rather by the longest chain of
dependencies that it needs to wait on. In many cases `serde` is part of
that long chain, as it is part of a long chain if the `derive` feature
is active:

`proc-macro2` compile build script > `proc-macro2` run build script >
`proc-macro2` > `quote` > `syn` > `serde_derive` > `serde` >
`serde_json` (or any crate that depends on serde)

By decoupling it from `serde_derive`, the chain is shortened and compile
times get much better.

Check this issue for a deeper elaboration:
serde-rs/serde#2584

For `wasmtime` I'm seeing a reduction from 24.75s to 22.45s when
compiling in `release` mode. This is because wasmtime through `gimli`
has a dependency on `indexmap` which can only start compiling when
`serde` is finished, which you want to happen as early as possible so
some of wasmtime's dependencies can start compiling.

To measure the full effect, the dependencies can't by themselves
activate the `derive` feature. I've upstreamed a patch for
`fxprof-processed-profile` which was the only dependency that activated
it for `wasmtime` (not yet published to crates.io). `wasmtime-cli` and
co. may need patches for their dependencies to see a similar
improvement.
alexcrichton added a commit to alexcrichton/wasm-tools that referenced this pull request Oct 6, 2023
I noticed recently that `wit-component` was relatively slow to compile
and it's going to be an introductory requirement for any Rust crates so
I wanted to optimize its build a little bit. This is the first
optimization which is to disable the `derive` feature of the `serde`
crate to enable build build pipelining opportunities.

This is the same as bytecodealliance/wasmtime#6917
alexcrichton added a commit to bytecodealliance/wasm-tools that referenced this pull request Oct 6, 2023
* Use `serde_derive` instead of the `derive` feature

I noticed recently that `wit-component` was relatively slow to compile
and it's going to be an introductory requirement for any Rust crates so
I wanted to optimize its build a little bit. This is the first
optimization which is to disable the `derive` feature of the `serde`
crate to enable build build pipelining opportunities.

This is the same as bytecodealliance/wasmtime#6917

* Fix build issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cranelift:meta Everything related to the meta-language. cranelift Issues related to the Cranelift code generator
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants