Import SourceFiles from crate before decoding foreign Span

[Aaron1011]

Fixes #92163
Fixes #92014

When writing to the incremental cache, we encode all Spans
we encounter, regardless of whether or not their SourceFile
comes from the local crate, or from a foreign crate.

When we decode a Span, we use the StableSourceFileId we encoded
to locate the matching SourceFile in the current session. If this
id corresponds to a SourceFile from another crate, then we need to
have already imported that SourceFile into our current session.

This usually happens automatically during resolution / macro expansion,
when we try to resolve definitions from other crates. In certain cases,
however, we may try to load a Span from a transitive dependency
without having ever imported the SourceFiles from that crate, leading
to an ICE.

This PR fixes the issue by enconding the SourceFile's CrateNum
when we encode a Span. During decoding, we call imported_source_files()
when we encounter a foreign CrateNum, which ensure that all
SourceFiles from that crate are imported into the current session.

[rust-highfive]

r? @matthewjasper

(rust-highfive has picked a reviewer for you, use r? to override)

[Aaron1011]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

[bors]

⌛ Trying commit 694cc860db597055044b55e35e074a9f5304e8d0 with merge b03e9a396b02e7285c97d453d8c0d59c7fd4d43e...

[bors]

☀️ Try build successful - checks-actions
Build commit: b03e9a396b02e7285c97d453d8c0d59c7fd4d43e (b03e9a396b02e7285c97d453d8c0d59c7fd4d43e)

[rust-timer]

Queued b03e9a396b02e7285c97d453d8c0d59c7fd4d43e with parent e100ec5, future comparison URL.

[rust-timer]

Finished benchmarking commit (b03e9a396b02e7285c97d453d8c0d59c7fd4d43e): comparison url.

Summary: This change led to very large relevant mixed results 🤷 in compiler performance.

• Very large improvement in instruction counts (up to -14.5% on incr-full builds of webrender-wrench)
• Large regression in instruction counts (up to 4.6% on incr-unchanged builds of tuple-stress)

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 try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf +perf-regression

[Aaron1011]

I think the 'improvement' is just due to the fact that that benchmark was previously broken.

[michaelwoerister]

I'm tempted to r+ this but the performance regression doesn't look too good. A CrateNum is encoded as a 16-byte stable hash, if I remember correctly. It's not unexpected that adding 16 bytes to each encoded span would show up in benchmarks.

EncodedSourceFileId::translate also seems to reconstruct the CrateNum for a given source file. Can you use that to import source files before accessing the SourceMap?

[Aaron1011]

@michaelwoerister: Thanks, I didn't notice that. I've updated the PR to use the already-decoded CrateNum.

[Aaron1011]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

[bors]

⌛ Trying commit d922092 with merge 529fa20023160e522151496a1a7fffc7b07261d1...

[bors]

☀️ Try build successful - checks-actions
Build commit: 529fa20023160e522151496a1a7fffc7b07261d1 (529fa20023160e522151496a1a7fffc7b07261d1)

[rust-timer]

Queued 529fa20023160e522151496a1a7fffc7b07261d1 with parent 489296d, future comparison URL.

[rust-timer]

Finished benchmarking commit (529fa20023160e522151496a1a7fffc7b07261d1): comparison url.

Summary: This change led to very large relevant improvements 🎉 in compiler performance.

• Very large improvement in instruction counts (up to -14.6% on incr-full builds of webrender-wrench)

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.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression

[Aaron1011]

@michaelwoerister It looks like your suggested fixed the perf regression entirely!

[Aaron1011]

@michaelwoerister Do you think this is now good to merge? A large number of people are hitting this on the latest nightly.

[cjgillot]

The perf regression appears resolved. Let's go ahead and merge this.
r? @cjgillot
@bors r+

[bors]

📌 Commit d922092 has been approved by cjgillot

[bors]

⌛ Testing commit d922092 with merge 984a6bf...

[bors]

☀️ Test successful - checks-actions
Approved by: cjgillot
Pushing 984a6bf to master...

[rust-timer]

Finished benchmarking commit (984a6bf): comparison url.

Summary: This change led to very large relevant improvements 🎉 in compiler performance.

• Very large improvement in instruction counts (up to -14.5% on incr-full builds of webrender-wrench)

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

[michaelwoerister]

I'm glad the alternative approach worked out.
Thanks for taking on the review while I was offline during the holidays, @cjgillot!