Avoid duplicating code for each query #69808
Conversation
|
r? @estebank (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
@bors try @rust-timer queue |
|
Awaiting bors try build completion |
Avoid duplicating code for each query 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. The first part of this PR reduces the amount of code generic on the query, replacing it by code generic on the key-value types. I can split it out if needed. In a second part, the non-inlined methods in the `QueryAccessors` and `QueryDescription` traits are made into a virtual dispatch table. This allows to reduce even more the number of generated functions. This allows to save 1.5s on check build, and 10% on the size of the librustc.rlib. (Attributed roughly half and half). My computer is not good enough to measure properly compiling time. I have no idea of the effect on performance. A perf run may be required. cc #65031
|
☀️ Try build successful - checks-azure |
|
Queued 576b7b6 with parent 823ff8c, future comparison URL. |
|
Finished benchmarking try commit 576b7b6, comparison URL. |
|
Causes a slight regression in the compiler's performance, unfortunately. |
|
This seems to be very similar to my branch here. That branch goes a bit further with the removal of the This does seem to increase the compiler performance though, not sure if that's just perf noise or due to better instruction cache utilization. |
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
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
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
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
|
Rebased. @Zoxc: Is there something I should take from your branch and add here? |
|
@cjgillot I'd leave that for a future PR. @bors try @rust-timer queue |
|
Awaiting bors try build completion |
|
⌛ Trying commit 21349c0c7a78f5e13e5156f4f2edb4eb16400093 with merge d5a581b4e2aad68744b0a6138de00c4a572a1b85... |
|
☀️ Try build successful - checks-azure |
|
@rust-timer build d5a581b4e2aad68744b0a6138de00c4a572a1b85 |
|
Queued d5a581b4e2aad68744b0a6138de00c4a572a1b85 with parent 7900b2b, future comparison URL. |
joelpalmer
added
S-waiting-on-review
joelpalmer
removed
the
S-waiting-on-review
|
☔ The latest upstream changes (presumably #70674) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Rebased. |
| @@ -645,6 +647,7 @@ where | |||
| /// side-effects -- e.g., in order to report errors for erroneous programs. | |||
| /// | |||
| /// Note: The optimization is only available during incr. comp. | |||
| #[inline(never)] | |||
There was a problem hiding this comment.
why the #[inline(never)] marker here? Did you observe a need for it on some benchmark locally?
There was a problem hiding this comment.
(no, your description says your rig is not suitable for local benchmarking, so that cannot be the answer...)
There was a problem hiding this comment.
I don't remember. I can remove it.
| @@ -681,7 +684,8 @@ fn ensure_query_impl<CTX, C>( | |||
| } | |||
| } | |||
|
|
|||
| fn force_query_impl<C, CTX>( | |||
| #[inline(never)] | |||
There was a problem hiding this comment.
(same as above, just wondering what prompted this use of #[inline(never)]...)
|
This looks great. I am especially happy with how the commits in this PR were laid out; it made it very easy to review. |
|
@bors r+ rollup=never |
|
📌 Commit e4976d0 has been approved by |
bors
added
S-waiting-on-bors
|
(tagging as rollup=never since we want to track its performance impact separately from other PR's) |
|
@bors p=1 |
|
☀️ Test successful - checks-azure |
There are at the moment roughly 170 queries in librustc.
The way
ty::queryis structured, a lot of code is duplicated for each query.I suspect this to be responsible for a part of librustc'c compile time.
The first part of this PR reduces the amount of code generic on the query,
replacing it by code generic on the key-value types. I can split it out if needed.
In a second part, the non-inlined methods in the
QueryAccessorsandQueryDescriptiontraitsare made into a virtual dispatch table. This allows to reduce even more the number of generated
functions.
This allows to save 1.5s on check build, and 10% on the size of the librustc.rlib.
(Attributed roughly half and half).
My computer is not good enough to measure properly compiling time.
I have no idea of the effect on performance. A perf run may be required.
cc #65031