-
Notifications
You must be signed in to change notification settings - Fork 12.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Investigate performance impact of debuginfo typename generation #86431
Labels
A-debuginfo
Area: Debugging information in compiled programs (DWARF, PDB, etc.)
C-enhancement
Category: An issue proposing an enhancement or a PR with one.
I-compiletime
Issue: Problems and improvements with respect to compile times.
Comments
michaelwoerister
added
A-debuginfo
Area: Debugging information in compiled programs (DWARF, PDB, etc.)
C-enhancement
Category: An issue proposing an enhancement or a PR with one.
I-compiletime
Issue: Problems and improvements with respect to compile times.
labels
Jun 18, 2021
#84873 (comment) is also somewhat related, the names getting passed into llvm can get very long, especially for generators |
bors
added a commit
to rust-lang-ci/rust
that referenced
this issue
Jul 19, 2021
…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.
2 tasks
#88898 implemented caching as an experiment but it didn't really improve performance. We concluded that we'd rather not have the additional implementation complexity. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
A-debuginfo
Area: Debugging information in compiled programs (DWARF, PDB, etc.)
C-enhancement
Category: An issue proposing an enhancement or a PR with one.
I-compiletime
Issue: Problems and improvements with respect to compile times.
The code generating type names for debuginfo does not seem to do any kind of caching of repeatedly occurring string segments (e.g. path prefixes and type renderings). Given that type names can get very long when generics are involved it's worth investigating how much time the compiler spends doing redundant work here.
The text was updated successfully, but these errors were encountered: