Uh oh!
There was an error while loading. Please reload this page.
ast: Remove some indirection layers from values in key-value attributes - #80441
Conversation
rust-highfive
commented
Dec 28, 2020
(rust-highfive has picked a reviewer for you, use r? to override) |
petrochenkov
commented
Dec 28, 2020
@bors try @rust-timer queue |
rust-timer
commented
Dec 28, 2020
Awaiting bors try build completion. |
bors
commented
Dec 28, 2020
⌛ Trying commit 4e34c7d7e884f56e81d2f73b969a4eafbbbd0355 with merge 702d7ce67a06c676371e5a9417e2172e3e0540f6... |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
petrochenkov
commented
Dec 28, 2020
@bors try @rust-timer queue |
rust-timer
commented
Dec 28, 2020
Awaiting bors try build completion. |
bors
commented
Dec 28, 2020
⌛ Trying commit d29c28eee068bb08e02782975524e0de57defe7d with merge e3c34b4b68e99fdff4c3f40e35e9df57d62b0085... |
bors
commented
Dec 28, 2020
☀️ Try build successful - checks-actions |
rust-timer
commented
Dec 28, 2020
Queued e3c34b4b68e99fdff4c3f40e35e9df57d62b0085 with parent 76aca66, future comparison URL. @rustbot label: +S-waiting-on-perf |
rust-timer
commented
Dec 28, 2020
Finished benchmarking try commit (e3c34b4b68e99fdff4c3f40e35e9df57d62b0085): comparison url. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up. @bors rollup=never |
petrochenkov
commented
Jan 9, 2021
r? @Aaron1011 |
Uh oh!
There was an error while loading. Please reload this page.
Aaron1011
commented
Jan 9, 2021
@petrochenkov: Can you rebase against master? |
petrochenkov
commented
Jan 9, 2021
Rebased. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Aaron1011
commented
Jan 9, 2021
r=me with the two comments addressed. |
petrochenkov
commented
Jan 9, 2021
@bors r=Aaron1011 |
bors
commented
Jan 9, 2021
📌 Commit 71cd6f4 has been approved by |
bors
commented
Jan 9, 2021
bors
commented
Jan 10, 2021
☀️ Test successful - checks-actions |
rylev
commented
Jan 12, 2021
@petrochenkov@Aaron1011 It seems that this change caused some perf regressions. Given that the perf run you ran above yielded only positive results, this is likely due to some other change getting in before this landed that introduced perf gains which this PR subsequently reversed. It looks like the largest perf hit was to the Additionally, it's not clear to me how this change helped addressed the original perf regression as the impacted benchmarks were not improved by this PR. Would you mind clarifying that? |
I'm ok with reverting this PR if it causes mixed results, I wouldn't say it brings any improvements besides the perf effect.
#78837 introduced more indirection by boxing literals as interpolated expressions, this PR removed some indirection by keeping a single token instead of a The ultimate solution to the regressions from #78837 is to keep AST on arena (like it's already done with HIR), so that no expressions are ever boxed. That would help not only expressions in key-value attributes, but all expressions in functions as well (which are in majority). |
Trying to address some perf regressions from #78837 (comment).