Skip to content
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

incr. comp.: Access span information more carefully throughout the compiler. #43088

Open
michaelwoerister opened this issue Jul 6, 2017 · 0 comments
Labels
A-incr-comp Area: Incremental compilation C-cleanup Category: PRs that clean code up or issues documenting cleanup. I-compiletime Issue: Problems and improvements with respect to compile times. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@michaelwoerister
Copy link
Member

Currently span information is accessed and copied rather haphazardly throughout the compiler which introduces lots of "false" dependency edges, because the span information is then never used or just used for error reporting. A few examples are:

  • The code generating debuginfo for structs, enums, and unions fetches the def_span of the type in questions, just to throw it away again immediately.
  • Queries collect spans from all over for possible cycle error reporting. In many cases these would not have to be access otherwise.
  • MIR contains span information which is later threaded through to debuginfo, which forces us to have manual special casing for ICH computation depending on debuginfo being enabled or not.
  • Type inference seems to pre-collect span information for error reporting. This information will not be used unless an actual error occurs.

I'm sure there are many other examples. Since spans are inherently unstable (adding a new-line somewhere will change all subsequent spans within the same file) this is a problem.

One possible solution would be to not store spans directly in the HIR and MIR (and other places) anymore but in a side-table that is part of the HIR map. Instead of a Span one would use and pass around a SpanId (which can just be a newtyped HirId or NodeId). Only when the SpanId is resolved to an actual Span, would we register a dependency on the thing the span originated from.

cc @nikomatsakis

@michaelwoerister michaelwoerister added A-incr-comp Area: Incremental compilation I-slow Issue: Problems and improvements with respect to performance of generated code. labels Jul 6, 2017
@Mark-Simulacrum Mark-Simulacrum added I-compiletime Issue: Problems and improvements with respect to compile times. and removed I-slow Issue: Problems and improvements with respect to performance of generated code. labels Jul 7, 2017
bors added a commit that referenced this issue Jul 12, 2017
…komatsakis

incr.comp.: Don't include span information in the ICH of type definitions

This should improve some of the `regex` tests on perf.rlo. Not including spans into the ICH is harmless until we also cache warnings. To really solve the problem, we need to do more refactoring (see #43088).

r? @nikomatsakis
@Mark-Simulacrum Mark-Simulacrum added the C-cleanup Category: PRs that clean code up or issues documenting cleanup. label Jul 28, 2017
bors added a commit that referenced this issue Aug 30, 2017
Make fields of `Span` private

I actually tried to intern spans and benchmark the result<sup>*</sup>, and this was a prerequisite.
This kind of encapsulation will be a prerequisite for any other attempt to compress span's representation, so I decided to submit this change alone.

The issue #43088 seems relevant, but it looks like `SpanId` won't be able to reuse this interface, unless the tables are global (like interner that I tried) and are not a part of HIR.
r? @michaelwoerister anyway

<sup>*</sup> Interning means 2-3 times more space is required for a single span, but duplicates are free. In practice it turned out that duplicates are not *that* common, so more memory was wasted by interning rather than saved.
@jonas-schievink jonas-schievink added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jan 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-incr-comp Area: Incremental compilation C-cleanup Category: PRs that clean code up or issues documenting cleanup. I-compiletime Issue: Problems and improvements with respect to compile times. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants