Skip to content

Slow compilation of actix-web routes #140944

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

Open
Nutomic opened this issue May 12, 2025 · 3 comments
Open

Slow compilation of actix-web routes #140944

Nutomic opened this issue May 12, 2025 · 3 comments
Labels
C-bug Category: This is a bug. E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example I-compiletime Issue: Problems and improvements with respect to compile times. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Nutomic
Copy link

Nutomic commented May 12, 2025

I noticed that Rust is extremely slow to compile a rather simple file containing route definitions for the actix-web webserver. It has 200 lines of imports, 250 lines of code and no macros, yet takes ~60s to compile. Duration is the same on Rust 1.81, 1.86 and nightly-2025-04-27.

Here is an excerpt from the profiling data, and you can also view the .mm_profdata.

Item Self time % of total time Time Item count Incremental result hashing time
layout_of 29.25s 42.066 111.38s 123703 32.73ms
normalize_canonicalized_projection_ty 17.28s 24.851 18.07s 15554 5.31ms
codegen_select_candidate 9.16s 13.173 9.39s 28989 9.75ms
LLVM_module_codegen_emit_obj 5.59s 8.042 5.59s 256 0.00ns
LLVM_passes 2.13s 3.069 2.13s 1 0.00ns
codegen_module 1.40s 2.009 4.43s 256 0.00ns
evaluate_obligation 951.66ms 1.369 1.03s 47932 7.69ms
items_of_instance 301.15ms 0.433 56.05s 57443 34.90ms

Whats strange is that the same file on the main branch takes only ~25s to compile (which is still too long). There it is part of the main binary crate.

To reproduce run:

git clone https://github.com/LemmyNet/lemmy.git --recursive --branch api-routes-crate
cd lemmy
cargo build --timings -p lemmy_api_routes
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label May 12, 2025
@jieyouxu jieyouxu added I-compiletime Issue: Problems and improvements with respect to compile times. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example C-bug Category: This is a bug. labels May 12, 2025
@lqd
Copy link
Member

lqd commented May 13, 2025

The above is entirely coming from the layout sanity checks, which is very unexpected.

It looks to be taking a long time to compute uninhabitedness for some of the big types in this crate (big coroutines, a lot of the time), sometimes multiple seconds for a single type. Overall, locally with the latest master commit at the time, the layout sanity checks take 66 seconds with the type-level uninhabitedness check, compared to 220ms without (using try build artifacts of the proposed change below).

@RalfJung would it be acceptable if I moved this uninhabitedness assertion (added in #96872) a bit down, to be gated by cfg(debug_assertions) like the other expensive layout sanity checks in this function?

@RalfJung
Copy link
Member

Isn't that query-cached? If not, maybe it should be? Or are we really only calling is_privately_uninhabited there?

Cc @oli-obk @compiler-errors

@lqd
Copy link
Member

lqd commented May 14, 2025

The layout_of query is supposed to be cached, for both unnormalized and normalized types, AFAICT. What's being computed is unexpectedly slow (the is_privately_uninhabited check is causing the slowness), so caching doesn't help unfortunately.

Why the uninhabitedness check is slow is unclear (michael mentioned he may look at it), but these look to be very big coroutines (some of the big DB query types are also slow) and maybe some additional caching could help, but the easiest/lowest hanging fruit is likely treating this sanity check like the other expensive checks, as debug assertions.

(There seemingly are also other compilation performance regressions related to that crate, so there may other be compounding factors that make things worse. We'll know more soon about this)

@jieyouxu jieyouxu removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label May 15, 2025
bors added a commit to rust-lang-ci/rust that referenced this issue May 15, 2025
move expensive layout sanity check to debug assertions

It is [hard to fix](rust-lang#141006 (comment)) the slowness in the uninhabitedness computation for very big types but we can fix the very specific case of them being called during the layout sanity checks, as described in rust-lang#140944.

So this PR moves this uninhabitedness check to the other expensive layout sanity checks that are ran under `debug_assertions`.

It makes building the `lemmy_api_routes` crate's self-profile `layout_of` query go from

```
+--------------------------------------------------------+-----------+-----------------+----------+------------+---------------------------------+
| Item                                                   | Self time | % of total time | Time     | Item count | Incremental result hashing time |
+--------------------------------------------------------+-----------+-----------------+----------+------------+---------------------------------+
| layout_of                                              | 63.02s    | 41.895          | 244.26s  | 123703     | 50.30ms                         |
+--------------------------------------------------------+-----------+-----------------+----------+------------+---------------------------------+
```

on master (2m17s total), to

```
| layout_of                                              | 330.21ms  | 0.372           | 26.90s   | 123703     | 53.19ms                         |
```

with this PR (1m15s total).

(Note that the [perf run results](rust-lang#141039 (comment)) below look a bit better than [an earlier run](https://perf.rust-lang.org/compare.html?start=4eca99a18eab3d4e28ed1ce3ee620d442955a470&end=c4a00993f8ee02c7565e7be652608817ea2fb97d&stat=instructions:u) I did in another PR. There may be some positive noise there, or post-merge results could differ a bit)

Since we discussed this today, r? `@compiler-errors` — and cc `@lcnr` and `@RalfJung.`
bors added a commit to rust-lang-ci/rust that referenced this issue May 18, 2025
move expensive layout sanity check to debug assertions

It is [hard to fix](rust-lang#141006 (comment)) the slowness in the uninhabitedness computation for very big types but we can fix the very specific case of them being called during the layout sanity checks, as described in rust-lang#140944.

So this PR moves this uninhabitedness check to the other expensive layout sanity checks that are ran under `debug_assertions`.

It makes building the `lemmy_api_routes` crate's self-profile `layout_of` query go from

```
+--------------------------------------------------------+-----------+-----------------+----------+------------+---------------------------------+
| Item                                                   | Self time | % of total time | Time     | Item count | Incremental result hashing time |
+--------------------------------------------------------+-----------+-----------------+----------+------------+---------------------------------+
| layout_of                                              | 63.02s    | 41.895          | 244.26s  | 123703     | 50.30ms                         |
+--------------------------------------------------------+-----------+-----------------+----------+------------+---------------------------------+
```

on master (2m17s total), to

```
| layout_of                                              | 330.21ms  | 0.372           | 26.90s   | 123703     | 53.19ms                         |
```

with this PR (1m15s total).

(Note that the [perf run results](rust-lang#141039 (comment)) below look a bit better than [an earlier run](https://perf.rust-lang.org/compare.html?start=4eca99a18eab3d4e28ed1ce3ee620d442955a470&end=c4a00993f8ee02c7565e7be652608817ea2fb97d&stat=instructions:u) I did in another PR. There may be some positive noise there, or post-merge results could differ a bit)

Since we discussed this today, r? `@compiler-errors` — and cc `@lcnr` and `@RalfJung.`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example I-compiletime Issue: Problems and improvements with respect to compile times. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants