Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.
[debuginfo] Emit associated type bindings in trait object type names. #87153
Conversation
|
(rust-highfive has picked a reviewer for you, use r? to override) |
|
r? @ghost |
|
@bors try @rust-timer queue |
|
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
…trait-projection-bounds, r=<try> (wip) [debuginfo] Emit associated type bindings in trait object type names. This PR message is a stub. Opening early for starting a perf run. Fixes rust-lang#86134.
This comment has been hidden.
This comment has been hidden.
This comment has been hidden.
This comment has been hidden.
|
|
michaelwoerister
force-pushed
the
debuginfo-names-dyn-trait-projection-bounds
branch
from
2e51462
to
8fa22dd
|
@bors try @rust-timer queue |
|
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
…trait-projection-bounds, r=<try> (wip) [debuginfo] Emit associated type bindings in trait object type names. This PR message is a stub. Opening early for starting a perf run. Fixes rust-lang#86134.
|
|
|
Queued 585e91c with parent b919797, future comparison URL. |
|
Finished benchmarking try commit (585e91c): comparison url. Summary: This change led to significant regressions
If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR led to changes in compiler perf. Next Steps: If you can justify the regressions found in this perf run, please indicate this with @bors rollup=never |
michaelwoerister
changed the title
…trait-projection-bounds, r=wesleywiser [debuginfo] Emit associated type bindings in trait object type names. This PR updates debuginfo type name generation for trait objects to include associated type bindings and auto trait bounds -- so that, for example, the debuginfo type name of `&dyn Iterator<Item=Foo>` and `&dyn Iterator<Item=Bar>` don't both map to just `&dyn Iterator` anymore. The following table shows examples of debuginfo type names before and after the PR: | type | before | after | |------|---------|-------| | `&dyn Iterator<Item=u32>>` | `&dyn Iterator` | `&dyn Iterator<Item=u32>` | | `&(dyn Iterator<Item=u32>> + Sync)` | `&dyn Iterator` | `&(dyn Iterator<Item=u32> + Sync)` | | `&(dyn SomeTrait<bool, i8, Bar=u32>> + Send)` | `&dyn SomeTrait<bool, i8>` | `&(dyn SomeTrait<bool, i8, Bar=u32>> + Send)` | For targets that need C++-like type names, we use `assoc$<Item,u32>` instead of `Item=u32`: | type | before | after | |------|---------|-------| | `&dyn Iterator<Item=u32>>` | `ref$<dyn$<Iterator> >` | `ref$<dyn$<Iterator<assoc$<Item,u32> > > >` | | `&(dyn Iterator<Item=u32>> + Sync)` | `ref$<dyn$<Iterator> >` | `ref$<dyn$<Iterator<assoc$<Item,u32> >,Sync> >` | | `&(dyn SomeTrait<bool, i8, Bar=u32>> + Send)` | `ref$<dyn$<SomeTrait<bool, i8> > >` | `ref$<dyn$<SomeTrait<bool,i8,assoc$<Bar,u32> > >,Send> >` | The PR also adds self-profiling measurements for debuginfo type name generation (re. rust-lang#86431). It looks like the compiler spends up to 0.5% of its time in that task, so the potential for optimizing it via caching seems limited. However, the perf run also shows [the biggest regression](https://perf.rust-lang.org/detailed-query.html?commit=585e91c718b0b2c5319e1fffd0ff1e62aaf7ccc2&base_commit=b9197978a90be6f7570741eabe2da175fec75375&benchmark=tokio-webpush-simple-debug&run_name=incr-unchanged) in a test case that does not even invoke the code in question. This suggests that the length of the names we generate here can affect performance by influencing how much data the linker has to copy around. Fixes rust-lang#86134.
This comment has been hidden.
This comment has been hidden.
|
|
michaelwoerister
force-pushed
the
debuginfo-names-dyn-trait-projection-bounds
branch
from
55820a3
to
1757542
|
@bors r=wesleywiser |
|
|
…trait-projection-bounds, r=wesleywiser [debuginfo] Emit associated type bindings in trait object type names. This PR updates debuginfo type name generation for trait objects to include associated type bindings and auto trait bounds -- so that, for example, the debuginfo type name of `&dyn Iterator<Item=Foo>` and `&dyn Iterator<Item=Bar>` don't both map to just `&dyn Iterator` anymore. The following table shows examples of debuginfo type names before and after the PR: | type | before | after | |------|---------|-------| | `&dyn Iterator<Item=u32>>` | `&dyn Iterator` | `&dyn Iterator<Item=u32>` | | `&(dyn Iterator<Item=u32>> + Sync)` | `&dyn Iterator` | `&(dyn Iterator<Item=u32> + Sync)` | | `&(dyn SomeTrait<bool, i8, Bar=u32>> + Send)` | `&dyn SomeTrait<bool, i8>` | `&(dyn SomeTrait<bool, i8, Bar=u32>> + Send)` | For targets that need C++-like type names, we use `assoc$<Item,u32>` instead of `Item=u32`: | type | before | after | |------|---------|-------| | `&dyn Iterator<Item=u32>>` | `ref$<dyn$<Iterator> >` | `ref$<dyn$<Iterator<assoc$<Item,u32> > > >` | | `&(dyn Iterator<Item=u32>> + Sync)` | `ref$<dyn$<Iterator> >` | `ref$<dyn$<Iterator<assoc$<Item,u32> >,Sync> >` | | `&(dyn SomeTrait<bool, i8, Bar=u32>> + Send)` | `ref$<dyn$<SomeTrait<bool, i8> > >` | `ref$<dyn$<SomeTrait<bool,i8,assoc$<Bar,u32> > >,Send> >` | The PR also adds self-profiling measurements for debuginfo type name generation (re. rust-lang#86431). It looks like the compiler spends up to 0.5% of its time in that task, so the potential for optimizing it via caching seems limited. However, the perf run also shows [the biggest regression](https://perf.rust-lang.org/detailed-query.html?commit=585e91c718b0b2c5319e1fffd0ff1e62aaf7ccc2&base_commit=b9197978a90be6f7570741eabe2da175fec75375&benchmark=tokio-webpush-simple-debug&run_name=incr-unchanged) in a test case that does not even invoke the code in question. This suggests that the length of the names we generate here can affect performance by influencing how much data the linker has to copy around. Fixes rust-lang#86134.
This comment has been hidden.
This comment has been hidden.
|
|
…ginfo type names.
michaelwoerister
force-pushed
the
debuginfo-names-dyn-trait-projection-bounds
branch
from
1757542
to
5b1bfae
|
@bors r=wesleywiser |
|
|
|
|
![@github-actions[bot]](https://web.archive.org/web/20210909190023im_/https://avatars.githubusercontent.com/in/15368?v=4){kind=small}
|
Given the comments on how one might address this performance issue, and the fact that it doesn't seem to have caused performance regressions after merging, I'm going to add the @rustbot label +perf-regression-triaged |
This PR updates debuginfo type name generation for trait objects to include associated type bindings and auto trait bounds -- so that, for example, the debuginfo type name of
&dyn Iterator<Item=Foo>and&dyn Iterator<Item=Bar>don't both map to just&dyn Iteratoranymore.The following table shows examples of debuginfo type names before and after the PR:
&dyn Iterator<Item=u32>>&dyn Iterator&dyn Iterator<Item=u32>&(dyn Iterator<Item=u32>> + Sync)&dyn Iterator&(dyn Iterator<Item=u32> + Sync)&(dyn SomeTrait<bool, i8, Bar=u32>> + Send)&dyn SomeTrait<bool, i8>&(dyn SomeTrait<bool, i8, Bar=u32>> + Send)For targets that need C++-like type names, we use
assoc$<Item,u32>instead ofItem=u32:&dyn Iterator<Item=u32>>ref$<dyn$<Iterator> >ref$<dyn$<Iterator<assoc$<Item,u32> > > >&(dyn Iterator<Item=u32>> + Sync)ref$<dyn$<Iterator> >ref$<dyn$<Iterator<assoc$<Item,u32> >,Sync> >&(dyn SomeTrait<bool, i8, Bar=u32>> + Send)ref$<dyn$<SomeTrait<bool, i8> > >ref$<dyn$<SomeTrait<bool,i8,assoc$<Bar,u32> > >,Send> >The PR also adds self-profiling measurements for debuginfo type name generation (re. #86431). It looks like the compiler spends up to 0.5% of its time in that task, so the potential for optimizing it via caching seems limited.
However, the perf run also shows the biggest regression in a test case that does not even invoke the code in question. This suggests that the length of the names we generate here can affect performance by influencing how much data the linker has to copy around.
Fixes #86134.
The text was updated successfully, but these errors were encountered: