Slow compile times / slimming down dependencies

[chinedufn]

Hey!

Background

I am looking to compile a dynamic library that does some CSS parsing.

• The host passes a CSS string to the css dylib.

• The css dylib parses the CSS and tracks some information about it internally.

• The css dylib returns a modified CSS string to the host.

---

I am looking to use an existing crate to handle this use case.

I am currently evaluating lightningcss and cssparser.

The Problem

I started writing my simple dylib using cssparser, but quickly found that it was too low level for my needs.

After searching on crates.io I found lightningcss. It was the perfect level of abstraction. I whipped up my css parsing dylib in no time. Great.

A day later I noticed that my builds had gotten slower since switching out cssparser for lightningcss.

Reproducing

Here's how to see the difference in compile time impact between cssparser and lightningcss.

https://github.com/chinedufn/rust-cssparser-and-lightningcss-compile-times

git clone git@github.com:chinedufn/rust-cssparser-and-lightningcss-compile-times.git
./run.sh

Example Output

rust-cssparser debug build timing
real 0m11.188s user 0m29.876s sys 0m3.621s

lightningcss debug build timing
real 0m40.468s user 2m40.552s sys 0m16.839s

Potential Solutions

I looked through lightningcss's Cargo.toml to see if any crates could be made optional.

Here are ones that immediately stood out:

# Stood out as deps that we could potentially make optional

serde = { version = "1.0.123", features = ["derive"] }
parcel_sourcemap = { version = "2.1.1", features = ["json"] }
rayon = "1.5.1"
dashmap = "5.0.0"

# I didn't look into what this is used for, but putting it in here since it depends on the
# heavy `syn` crate.
lightningcss-derive = "..."

---

Here are ones that could potentially be removed entirely:

# itertools is barely used and does not seem necessary
itertools = "0.10.1"
# Looks like we could easily copy/paste what we're using.
pathdiff = "0.2.1"

# I didn't look at all of the dependencies.. but it seems like there are a few that are barely used
# and could be good candidates for removing and just copy/pasting the tiny bit of code that
# we actually depend on.

---

Additionally, I wonder if we can turn default-features = false for lightningcss-derive's syn dependency
and only enable exactly what we're using.

Alternative Solutions

• Exposing the css string -> css rules code as its own crate. Not sure how desirable/feasible that is.

Questions

• Are you open to trimming down the dependencies?

• What other ways could we reduce compile times? Are there places where lightningcss uses lots of macros and/or generics that could be simplified?

• Is there anything that I'm missing? Are the slower compile times unavoidable?

Thank You

lightningcss was very easy to try out, so thanks!

[devongovett]

Yeah, lightningcss is some 60,000+ lines of code on top of cssparser so it makes sense that it is slower to compile. Would be awesome to make it faster though.

I think we could put the bundler behind a feature flag. That would remove rayon and dash map I think. We can at least see if it improves things.

Serde is mostly behind a flag already.

I would probably run cargo with the -Z timings option to see what is slow to compile. Or try -Z self-profile to dig in deeper. Would be nice to see where the compiler is spending it's time rather than guessing.

[chinedufn]

Serde is mostly behind a flag already.

Still needs to be compiled in full, so it'll hurt compilation times unless it's removed from the dep tree.

Would be nice to see where the compiler is spending it's time rather than guessing.

For sure. Running cargo llvm-lines > lines.txt on the lightningcss crate may be of use here.

---

Here is the output of cargo build --timings on the lightningcss crate

# To reproduce timings
git clone git@github.com:parcel-bundler/lightningcss.git
cd lightningcss
cargo +nightly build --timings
open target/cargo-timings/cargo-timing.html

Confirms that being able to avoid compiling huge dependencies (syn, rayon, etc) would cut down compile time substantially.

I think we could put the bundler behind a feature flag. That would remove rayon and dash map I think. We can at least see if it improves things.

Makes sense to me!

[chinedufn]

Submitted a few PRs.

I'd like to submit another batch after getting your feedback on the following:

---

I want to submit PR that puts parcel_sourcemap behind a "sourcemap" feature flag. This and the serde flag in #360 would make it possible to completely disable serde, serde_json and rkyv (among others).

---

I'd also like to submit a PR that makes lightningcss-derive optional behind a "visitor-derive" feature flag.

This, plus the rest of the PRs would be the final step in getting syn out of the dep tree for those that don't need it.

Well.. not entirely sure about syn.. there might be other things pulling it in.. I need to look at the dep tree more carefully...

---

I'd also like to submit a PR that removes derive_more from the parcel_selectors crate. It looks like we only use it on the Specificity type. Should be easy add a hand written impl.

---

Would these PRs be alright with you?

Would you want the sourcemap and lightningcss-derive features enabled by default?

---

Any hesitations here on your end? I'm happy to address whatever is needed to land these.
This batch plus the first batch should cut down compile times substantially, based on cargo build --timings.

[devongovett]

Thanks, I'll take a look soon. How much has it helped so far? From the timings, it looked like the lightningcss crate itself was most of the time.

[chinedufn]

Yeah looks like lightningcss is about 50% and then deps are the other 50%.

I'm focusing on deps first. Once we exhaust all savings there I'd love to help see if there are any potential wins inside of lightningcss's own code that don't hurt performance.

For context, my main motivation here is that I want to be able to use lightningcss's Rust API in a few more places in my codebase.

---

Only small savings so far. About 5%.

When I combine #358 #360 and #361 into a branch and time a build after cleaning I see about 39 seconds. Before any changes I was seeing about 41 seconds. I'm compiling with 8 cores.

The bigger deps are still in the tree. I expect us to see bigger savings when we can make them entirely optional.
The second batch of PRs described above should help.

Thanks, I'll take a look soon.

Awesome, no rush at all on my end. Thanks so much!

[devongovett]

Your other suggestions sound good btw. I think sourcemaps should be on by default (would be pretty commonly used), but visitors could be off by default (more rare). Thanks so much for working on this!

[chinedufn]

Awesome. #367 handles visitors. After #360 and #367 land I can submit a PR for sourcemaps.