Avoid query type in generics #69910

cjgillot · 2020-03-11T07:55:47Z

There are at the moment roughly 170 queries in librustc.
The way ty::query is structured, a lot of code is duplicated for each query.
I suspect this to be responsible for a part of librustc'c compile time.

This PR reduces the amount of code generic on the query,
replacing it by code generic on the key-value types.

This is split out of #69808,
and should not contain the perf regression.

cc #65031

rust-highfive · 2020-03-11T07:55:50Z

r? @eddyb

(rust_highfive has picked a reviewer for you, use r? to override)

Centril · 2020-03-11T08:24:15Z

r? @Zoxc

src/librustc/ty/query/plumbing.rs

Zoxc · 2020-03-11T13:28:03Z

I'm not sure how many unique key / value pairs we have for queries, if that isn't much lower than the total query count, this change isn't too impactful.

cjgillot · 2020-03-11T16:30:26Z

As a crude measurement, the number of instances of alloc_self_profile_query_strings_for_query_cache in the rlib goes from 201 to 148. QueryState::iter_results and <DefaultCache as QueryCache>::iter are called several times with closures, so go from 802 to 590 instances each.

Zoxc · 2020-03-12T07:15:19Z

That suggests that 26% of query key / value pairs are duplicates, so this is probably worthwhile.

src/librustc/ty/query/plumbing.rs

Zoxc · 2020-03-13T07:33:17Z

Why is my Erase trait added here?

cjgillot · 2020-03-13T07:48:54Z

I was experimenting with it yesterday, and somehow it ended up on the wrong branch. Sorry.

Zoxc · 2020-03-13T17:24:24Z

@bors try @rust-timer queue

rust-timer · 2020-03-13T17:24:26Z

Awaiting bors try build completion

bors · 2020-03-13T17:24:36Z

⌛ Trying commit 2bf1353 with merge 4adbc79...

Avoid query type in generics There are at the moment roughly 170 queries in librustc. The way ty::query is structured, a lot of code is duplicated for each query. I suspect this to be responsible for a part of librustc'c compile time. This PR reduces the amount of code generic on the query, replacing it by code generic on the key-value types. This is split out of #69808, and should not contain the perf regression. cc #65031

bors · 2020-03-13T20:41:39Z

☀️ Try build successful - checks-azure
Build commit: 4adbc79 (4adbc79bec9c18982551d4c0a11ed917425171bb)

rust-timer · 2020-03-13T20:41:41Z

Queued 4adbc79 with parent 54b7d21, future comparison URL.

rust-timer · 2020-03-14T00:10:31Z

Finished benchmarking try commit 4adbc79, comparison URL.

Zoxc · 2020-03-15T10:54:54Z

src/librustc/ty/query/plumbing.rs

+    pub(super) fn try_collect_active_jobs(
+        &self,
+        kind: DepKind,
+        make_query: impl Fn(C::Key) -> Query<'tcx> + Copy,


You could use &dyn for make_query here.

Zoxc · 2020-03-15T11:04:00Z

There are at the moment roughly 170 queries in librustc.
The way ty::query is structured, a lot of code is duplicated for each query.
I suspect this to be responsible for a part of librustc'c compile time.

I measured the compilation impact of the query system to be 25% of librustc's compile time 3. February.

src/librustc/ty/query/caches.rs

bors · 2020-03-15T23:50:44Z

☔ The latest upstream changes (presumably #68944) made this pull request unmergeable. Please resolve the merge conflicts.

cjgillot · 2020-03-16T10:33:13Z

Rebased.

Zoxc · 2020-03-17T16:54:42Z

I'd leave out the Bundle key commits since they add additional clone calls, otherwise this looks good.

cjgillot · 2020-03-17T19:09:41Z

Done.

Zoxc · 2020-03-17T19:23:51Z

@bors r+

bors · 2020-03-17T19:23:53Z

📌 Commit 8aa1328 has been approved by Zoxc

Avoid query type in generics There are at the moment roughly 170 queries in librustc. The way ty::query is structured, a lot of code is duplicated for each query. I suspect this to be responsible for a part of librustc'c compile time. This PR reduces the amount of code generic on the query, replacing it by code generic on the key-value types. This is split out of rust-lang#69808, and should not contain the perf regression. cc rust-lang#65031

@ghost

Rollup of 6 pull requests Successful merges: - rust-lang#69497 (Don't unwind when hitting the macro expansion recursion limit) - rust-lang#69901 (add #[rustc_layout(debug)]) - rust-lang#69910 (Avoid query type in generics) - rust-lang#69955 (Fix abort-on-eprintln during process shutdown) - rust-lang#70032 (put type params in front of const params in generics_of) - rust-lang#70119 (rustc: use LocalDefId instead of DefId in TypeckTables.) Failed merges: r? @ghost

Zoxc · 2020-03-24T04:34:38Z

src/librustc/ty/query/caches.rs

        &self,
-        state: &'tcx QueryState<'tcx, Q>,
+        state: &'tcx QueryState<'tcx, Self>,
        get_cache: GetCache,


Now that you connected up the types, you should be able to just get rid of get_cache: GetCache.

Zoxc · 2020-03-24T04:35:43Z

src/librustc/ty/query/plumbing.rs

+impl<'tcx, C: QueryCache> QueryState<'tcx, C> {
+    pub(super) fn get_lookup<K2: Hash>(
+        &'tcx self,
+        key: &K2,


K2 can now be C::Key.

@Zoxc

Move the query system to a dedicated crate The query system `rustc::ty::query` is split out into the `rustc_query_system` crate. Some commits are unformatted, to ease rebasing. Based on rust-lang#67761 and rust-lang#69910. r? @Zoxc

rust-highfive assigned eddyb Mar 11, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 11, 2020

rust-highfive assigned Zoxc and unassigned eddyb Mar 11, 2020

cjgillot force-pushed the polym branch from 93146b9 to 858d8fa Compare March 11, 2020 10:46

Zoxc reviewed Mar 11, 2020

View reviewed changes

src/librustc/ty/query/plumbing.rs Outdated Show resolved Hide resolved

Zoxc reviewed Mar 12, 2020

View reviewed changes

src/librustc/ty/query/plumbing.rs Outdated Show resolved Hide resolved

cjgillot force-pushed the polym branch from ca8fcc3 to c3c5e93 Compare March 12, 2020 22:22

Zoxc reviewed Mar 13, 2020

View reviewed changes

src/librustc/ty/query/plumbing.rs Outdated Show resolved Hide resolved

cjgillot force-pushed the polym branch from 11f03fa to 2bf1353 Compare March 13, 2020 07:48

Zoxc reviewed Mar 15, 2020

View reviewed changes

src/librustc/ty/query/caches.rs Outdated Show resolved Hide resolved

cjgillot force-pushed the polym branch from 3effbd6 to 474e8ba Compare March 15, 2020 13:18

Remove __query_compute module.

0bf5cae

cjgillot added 5 commits March 16, 2020 09:48

Remove Q parameter from try_get_cached.

5dc7c2e

Offload try_collect_active_jobs.

7d84f4f

Simplify type aliases.

7309b3c

Make QueryCache parameters associated types.

3abd475

Remove QueryState type alias.

5557407

cjgillot force-pushed the polym branch from 526236d to 2485ebf Compare March 16, 2020 10:22

Make stuff private.

8aa1328

cjgillot force-pushed the polym branch from 2485ebf to 8aa1328 Compare March 17, 2020 19:08

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 17, 2020

Centril mentioned this pull request Mar 19, 2020

Rollup of 5 pull requests #70141

Closed

cjgillot mentioned this pull request Mar 19, 2020

Move the query system to a dedicated crate #70162

Merged

Dylan-DPC-zz mentioned this pull request Mar 20, 2020

Rollup of 7 pull requests #70198

Closed

Dylan-DPC-zz mentioned this pull request Mar 21, 2020

Rollup of 6 pull requests #70222

Merged

bors merged commit 8deeac1 into rust-lang:master Mar 21, 2020

cjgillot deleted the polym branch March 21, 2020 17:52

Zoxc reviewed Mar 24, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Avoid query type in generics #69910

Avoid query type in generics #69910

cjgillot commented Mar 11, 2020

rust-highfive commented Mar 11, 2020

Centril commented Mar 11, 2020

Zoxc commented Mar 11, 2020

cjgillot commented Mar 11, 2020

Zoxc commented Mar 12, 2020

Zoxc commented Mar 13, 2020

cjgillot commented Mar 13, 2020

Zoxc commented Mar 13, 2020

rust-timer commented Mar 13, 2020

bors commented Mar 13, 2020

bors commented Mar 13, 2020

rust-timer commented Mar 13, 2020

rust-timer commented Mar 14, 2020

Zoxc Mar 15, 2020

Zoxc commented Mar 15, 2020

bors commented Mar 15, 2020

cjgillot commented Mar 16, 2020

Zoxc commented Mar 17, 2020

cjgillot commented Mar 17, 2020

Zoxc commented Mar 17, 2020

bors commented Mar 17, 2020

Zoxc Mar 24, 2020

Zoxc Mar 24, 2020

Avoid query type in generics #69910

Avoid query type in generics #69910

Conversation

cjgillot commented Mar 11, 2020

rust-highfive commented Mar 11, 2020

Centril commented Mar 11, 2020

Zoxc commented Mar 11, 2020

cjgillot commented Mar 11, 2020

Zoxc commented Mar 12, 2020

Zoxc commented Mar 13, 2020

cjgillot commented Mar 13, 2020

Zoxc commented Mar 13, 2020

rust-timer commented Mar 13, 2020

bors commented Mar 13, 2020

bors commented Mar 13, 2020

rust-timer commented Mar 13, 2020

rust-timer commented Mar 14, 2020

Zoxc Mar 15, 2020

Choose a reason for hiding this comment

Zoxc commented Mar 15, 2020

bors commented Mar 15, 2020

cjgillot commented Mar 16, 2020

Zoxc commented Mar 17, 2020

cjgillot commented Mar 17, 2020

Zoxc commented Mar 17, 2020

bors commented Mar 17, 2020

Zoxc Mar 24, 2020

Choose a reason for hiding this comment

Zoxc Mar 24, 2020

Choose a reason for hiding this comment