Test suite compilation spends a large amount of time in `evaluate_obligation`

[patrickfreed]

I originally posted this on the Rust forums but was directed here since the cause could be related to a compiler bug.

I'm looking for some help debugging why the compile times in the mongodb crate's test suite are much slower than the crate itself. For reference, a single incremental change can take a minute or longer, even when running just cargo check --tests. The crate itself compiles quickly--the issue is only with the tests.

The output of compiling with -Zself-profile indicates that nearly the entire time (~1m) is spent in evaluate_obligation, but I can't figure what this implies or how to possibly reduce this.

A few possible culprits that I've investigated so far:

• the tests make heavy use of #[cfg_attr(...)] to conditionally select either #[tokio::test] or #[async_std::test]. Removing all usages of this and defaulting to just #[tokio::test] has an insignificant impact on compile times
• the tests also make heavy use of a function that accepts a closure that returns a future. According to this thread, such functions can cause type length / compilation time issues, but converting it to a macro that calls a function directly didn't seem to change much. Perhaps that suffers from a similar issue?
• We generate a lot of builders via the typed-builder crate. Perhaps using them in the tests leads to long compilation times due to the generated builders having lots of generic parameters.
• this compiler issue related to async and compilation times, though someone posted their profile output and evaluate_obligation is not a factor in it.
• most of our tests are in src/test rather than tests, not sure if this has any impact

Anyways, I'm not sure where to go from here, and I was wondering if anyone else has encountered similar issues in the past or knows how I could further debug this to identify the root cause. Thanks!

Here are the top few results from the self-profile output:

+--------------------------------------------------+-----------+-----------------+----------+------------+-----------------------+
| Item                                             | Self time | % of total time | Time     | Item count | Incremental load time |
+--------------------------------------------------+-----------+-----------------+----------+------------+-----------------------+
| evaluate_obligation                              | 58.34s    | 85.300          | 58.39s   | 110480     | 0.00ns                |
+--------------------------------------------------+-----------+-----------------+----------+------------+-----------------------+
| expand_crate                                     | 3.54s     | 5.179           | 3.59s    | 1          | 0.00ns                |
+--------------------------------------------------+-----------+-----------------+----------+------------+-----------------------+
| typeck                                           | 2.74s     | 4.011           | 61.39s   | 1159       | 71.43ms               |
+--------------------------------------------------+-----------+-----------------+----------+------------+-----------------------+
| self_profile_alloc_query_strings                 | 979.78ms  | 1.433           | 980.49ms | 1          | 0.00ns                |
+--------------------------------------------------+-----------+-----------------+----------+------------+-----------------------+
| hir_lowering                                     | 204.80ms  | 0.299           | 204.80ms | 1          | 0.00ns                |
+--------------------------------------------------+-----------+-----------------+----------+------------+-----------------------+

[est31]

How many tests does the testsuite contain? Is it doctests? tests in the tests/ directory? #[test] functions?

Can you isolate the behaviour to a lone test that if you remove it, the time spent in evaluate_obligation is more normal again? You could try commenting out tests one by one. Maybe it is the result of the combination of multiple tests?

[jyn514]

We generate a lot of builders via the typed-builder crate. Perhaps using them in the tests leads to long compilation times due to the generated builders having lots of generic parameters.

Without looking into this for more than 30 seconds, I'd expect this to be the issue.

[jyn514]

For context, evaluate_obligation is the part of the compiler that determines whether a T: Trait bound is fulfilled.

[patrickfreed]

Can you isolate the behaviour to a lone test that if you remove it, the time spent in evaluate_obligation is more normal again? You could try commenting out tests one by one. Maybe it is the result of the combination of multiple tests?

I went ahead and commented each test out in chunks until they were all commented out and saw no meaningful change, which means the slowdown is somewhere in the test harness code rather than the tests themselves.

Digging further, I managed to find that it's likely due to a large number of of #[async_trait] implementations (which is in line with evaluate_obligation being related to determining if trait bounds are satisfied as @jyn514 pointed out). After commenting out most of the implementations, I was able to get the evaluate_obligation down to roughly 20s instead of a minute.

For reference, the trait looks like this:

#[async_trait]
pub trait TestOperation: Debug {
    async fn execute_on_collection(
        &self,
        collection: &Collection<Document>,
        session: Option<&mut ClientSession>,
    ) -> Result<Option<Bson>>;
    async fn execute_on_database(
        &self,
        database: &Database,
        session: Option<&mut ClientSession>,
    ) -> Result<Option<Bson>>;
    async fn execute_on_client(&self, client: &TestClient) -> Result<Option<Bson>>;
    async fn execute_on_session(&self, session: &mut ClientSession) -> Result<Option<Bson>>;
}

And we have maybe 60 implementations of that trait.

evaluate_obligation still takes up a big chunk of time after removing most of them, but I think it could be reduced further after removing that last few implementations of the trait.

self-profile output after commenting out:

+--------------------------------------------------+-----------+-----------------+----------+------------+-----------------------+
| Item                                             | Self time | % of total time | Time     | Item count | Incremental load time |
+--------------------------------------------------+-----------+-----------------+----------+------------+-----------------------+
| evaluate_obligation                              | 7.00s     | 31.556          | 7.18s    | 33687      | 0.00ns                |
+--------------------------------------------------+-----------+-----------------+----------+------------+-----------------------+
| expand_crate                                     | 5.28s     | 23.804          | 5.40s    | 1          | 0.00ns                |
+--------------------------------------------------+-----------+-----------------+----------+------------+-----------------------+
| typeck                                           | 2.39s     | 10.790          | 10.15s   | 1028       | 100.07ms              |
+--------------------------------------------------+-----------+-----------------+----------+------------+-----------------------+
| incr_comp_encode_dep_graph                       | 1.28s     | 5.761           | 1.28s    | 981842     | 0.00ns                |
+--------------------------------------------------+-----------+-----------------+----------+------------+-----------------------+

So does this seem like a bug in the compiler then? in async_trait? or neither?

[patrickfreed]

I'm happy to report that converting the async_trait implementations to regular trait implementations that return futures eliminated nearly all the time spent in evaluate_obligation. Given that, it seems like this may be a problem with async_trait, so I've filed an issue there.

edit: per @dtolnay, this is still likely a rustc issue, not an async_trait one.

[crepererum]

So we have this issue over at https://github.com/influxdata/influxdb_iox as well. Here is a (non-minimal) reproducible example:

$ git clone https://github.com/influxdata/influxdb_iox.git
$ cd influxdb_iox
$ git checkout 61ccdbba93b3908ca80ef1ecce8a0e96d89ef1f0
$ RUSTFLAGS="-Zself-profile -Zself-profile-events=default,args" cargo +nightly build
$ cd ..
$ summarize summarize query_tests-<PID>.mm_profdata

I've also followed https://blog.rust-lang.org/inside-rust/2020/02/25/intro-rustc-self-profile.html and created a chrome profile: https://we.tl/t-dqgMlqYWHI (can re-upload somewhere else on request)

There you can see the detailed calls to evaluate_obligation which often have these or similar parameters:

Canonical {
    max_universe: U0,
    variables: [
        CanonicalVarInfo { kind: Region(U0) }, 
        CanonicalVarInfo { kind: Region(U0) }
    ],
    value: ParamEnvAnd {
       param_env: ParamEnv {
           caller_bounds: [
               Binder(OutlivesPredicate(ReLateBound(DebruijnIndex(1), BoundRegion { var: 0, kind: BrAnon(0) }), ReLateBound(DebruijnIndex(1), BoundRegion { var: 1, kind: BrAnon(1) })), []),
               Binder(OutlivesPredicate(scenarios::delete::TwoDeleteMultiExprsFromRubOneMeasurementOneChunk, ReLateBound(DebruijnIndex(1), BoundRegion { var: 1, kind: BrAnon(1) })), [])
            ],
            reveal: UserFacing
        },
        value: Binder(TraitPredicate(<lifecycle::policy::LifecyclePolicy<server::db::lifecycle::WeakDb> as std::marker::Send>), [])
    }
}

Canonical {
    max_universe: U0,
    variables: [
        CanonicalVarInfo { kind: Region(U0) },
        CanonicalVarInfo { kind: Region(U0) }
    ],
    value: ParamEnvAnd {
        param_env: ParamEnv {
            caller_bounds: [
                Binder(OutlivesPredicate(ReLateBound(DebruijnIndex(1), BoundRegion { var: 0, kind: BrAnon(0) }), ReLateBound(DebruijnIndex(1), BoundRegion { var: 1, kind: BrAnon(1) })), []),
                Binder(OutlivesPredicate(scenarios::delete::TwoDeleteMultiExprsFromRubOneMeasurementOneChunk, ReLateBound(DebruijnIndex(1), BoundRegion { var: 1, kind: BrAnon(1) })), [])
            ],
        reveal: UserFacing
        },
        value: Binder(TraitPredicate(<std::sync::Arc<server::Db> as std::marker::Sync>), [])
    }
}

I'm not really sure how to read this, but could it be that the compiler is quite busy figuring out if a future is Send and Sync?

[est31]

It would be interesting to get a self contained example that only uses the async-trait crate, no other dependencies. Is that possible?

[crepererum]

I've tried, but was so far not able to reproduce it on a smaller scale. I THINK it's not just about the number of async-trait impls. I'll give it another shot and report once I have some smaller example.

[est31]

@crepererum yeah I thought about something like a crate that autogenerates a lot of async-trait impls.