Skip to content

Conversation

@Enselic
Copy link
Contributor

@Enselic Enselic commented May 31, 2025

Hi! Enabling the "derive" feature of serde means serde_derive must compile before serde. But serde does not depend on serde_derive. So this is an artifical and unnecessary dependency chain. Instead depend on serde_derive directly so serde and serde_derive (and its direct dependencies) can compile in parallel. That speeds up compilation by 22% (see below).

The maintainer of serde did the same change for bat a while ago: sharkdp/bat#2815

This does not only help camino build faster, but all downstream projects that directly or indirectly depend on camino. Without this fix, downstream projects can't speed up their builds in the same way since cargo features are additive.

Before

$ hyperfine --prepare 'cargo clean' 'cargo build --all-features'
Benchmark 1: cargo build --all-features
  Time (mean ± σ):      4.940 s ±  0.101 s    [User: 15.523 s, System: 3.558 s]
  Range (min … max):    4.773 s …  5.098 s    10 runs

image

After

$ hyperfine --prepare 'cargo clean' 'cargo build --all-features'
Benchmark 1: cargo build --all-features
  Time (mean ± σ):      3.831 s ±  0.138 s    [User: 16.185 s, System: 3.602 s]
  Range (min … max):    3.606 s …  4.040 s    10 runs

image

@Enselic Enselic force-pushed the speed-up branch 2 times, most recently from 92d0375 to d5bbb46 Compare May 31, 2025 18:51
@sunshowers
Copy link
Collaborator

sunshowers commented May 31, 2025

Thanks, yeah, I've been aware of this general strategy to parallelize builds. Will look at this and at the CI failure when I'm next at a computer.

@sunshowers
Copy link
Collaborator

sunshowers commented May 31, 2025

(The build failure is fixed.)

Looking at this in more detail, what do you think of just hand-writing the Serialize and Deserialize impls and not needing the derives at all? These aren't particularly complex impls.

Enabling the "derive" feature of `serde` means `serde_derive` must
compile before `serde`. But `serde` does not depend on `serde_derive`.
Instead depend on `serde_derive` directly so `serde` and `serde_derive`
can compile in parallel. That speeds up compilation by 22%:

Before:

    $ hyperfine --prepare 'cargo clean' 'cargo build --all-features'
    Benchmark 1: cargo build --all-features
      Time (mean ± σ):      4.940 s ±  0.101 s    [User: 15.523 s, System: 3.558 s]
      Range (min … max):    4.773 s …  5.098 s    10 runs

After:

    $ hyperfine --prepare 'cargo clean' 'cargo build --all-features'
    Benchmark 1: cargo build --all-features
      Time (mean ± σ):      3.831 s ±  0.138 s    [User: 16.185 s, System: 3.602 s]
      Range (min … max):    3.606 s …  4.040 s    10 runs
@Enselic
Copy link
Contributor Author

Enselic commented Jun 2, 2025

(Thanks!)

Since the use of serde_derive (directly or indirectly) is extremely widespread, getting rid of that dep completely in this crate still won't affect build times of downstream projects, I think. So I'd say it's not worth it.

@sunshowers
Copy link
Collaborator

Thanks. I think I'd like to drop serde_derive altogether anyway just because it's so straightforward. I'll land your change and then make another one on top to drop it.

@sunshowers sunshowers merged commit e6624c4 into camino-rs:main Jun 2, 2025
30 checks passed
sunshowers added a commit that referenced this pull request Jun 2, 2025
Continuing work from #101, we can just hand-write the `Utf8PathBuf` impl.

Before:

```
% hyperfine --prepare 'cargo clean' 'cargo build --features serde1'
Benchmark 1: cargo build --features serde1
  Time (mean ± σ):      2.454 s ±  0.028 s    [User: 3.928 s, System: 0.964 s]
  Range (min … max):    2.394 s …  2.484 s    10 runs
```

After:

```
% hyperfine --prepare 'cargo clean' 'cargo build --features serde1'
Benchmark 1: cargo build --features serde1
  Time (mean ± σ):      1.445 s ±  0.013 s    [User: 1.415 s, System: 0.406 s]
  Range (min … max):    1.427 s …  1.462 s    10 runs
```
sunshowers added a commit that referenced this pull request Jun 2, 2025
Continuing work from #101, we can just hand-write the `Utf8PathBuf` impl.

Before:

```
% hyperfine --prepare 'cargo clean' 'cargo build --features serde1'
Benchmark 1: cargo build --features serde1
  Time (mean ± σ):      2.454 s ±  0.028 s    [User: 3.928 s, System: 0.964 s]
  Range (min … max):    2.394 s …  2.484 s    10 runs
```

After:

```
% hyperfine --prepare 'cargo clean' 'cargo build --features serde1'
Benchmark 1: cargo build --features serde1
  Time (mean ± σ):      1.445 s ±  0.013 s    [User: 1.415 s, System: 0.406 s]
  Range (min … max):    1.427 s …  1.462 s    10 runs
```
sunshowers added a commit that referenced this pull request Jun 2, 2025
Continuing work from #101, we can just hand-write the `Utf8PathBuf` impl.

Before:

```
% hyperfine --prepare 'cargo clean' 'cargo build --features serde1'
Benchmark 1: cargo build --features serde1
  Time (mean ± σ):      2.454 s ±  0.028 s    [User: 3.928 s, System: 0.964 s]
  Range (min … max):    2.394 s …  2.484 s    10 runs
```

After:

```
% hyperfine --prepare 'cargo clean' 'cargo build --features serde1'
Benchmark 1: cargo build --features serde1
  Time (mean ± σ):      1.445 s ±  0.013 s    [User: 1.415 s, System: 0.406 s]
  Range (min … max):    1.427 s …  1.462 s    10 runs
```
@sunshowers
Copy link
Collaborator

Now out as camino-1.1.10. Thanks again!

@Enselic
Copy link
Contributor Author

Enselic commented Jun 3, 2025

(Sorry, missed that last comment.) Happy to help! Thanks for the quick release!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants