Resolver: Batched Import Resolution

[LorrensP-2158466]

View all comments

Transforms the current algorithm for resolving imports to a batched algorithm. Every import in the indeterminate_imports set is resolved in isolation. This is the only real difference from the current algorithm.

r? petrochenkov

[LorrensP-2158466]

Code looks a little better now and should be more correct than the previous commits, but I have yet to find a way to resolve the current test failures.

[LorrensP-2158466]

Okey, I did fix core not being built, but those other errors happened again. We now resolve the prelude import path when we create such an import at build_reduced_graph phase.

[petrochenkov]

Could you update the tests to make CI green, so I can see the difference?
(Even if the changes do not seem correct.)

[petrochenkov]

Moving prelude_import processing to build_reduced_graph may also be necessary for #139493.

[LorrensP-2158466]

I'll create a pr for it.

[LorrensP-2158466]

@rustbot ready

[petrochenkov]

@bors try @rust-timer queue

[rust-bors]

☀️ Try build successful (CI)
Build commit: fa4dd1f (fa4dd1f19bfeb3fd235dc7f1406c42404436bef5, parent: 59fd4ef94daa991e6797b5aa6127e824f3067def)

[rust-timer]

Finished benchmarking commit (fa4dd1f): comparison URL.

Overall result: ❌✅ regressions and improvements - no action needed

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

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

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	0.2%	[0.2%, 0.2%]	1
Regressions ❌ (secondary)	0.3%	[0.2%, 0.7%]	4
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.8%	[-0.8%, -0.8%]	1
All ❌✅ (primary)	0.2%	[0.2%, 0.2%]	1

Max RSS (memory usage)

Results (primary -0.0%, secondary 3.2%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	1.6%	[0.7%, 3.2%]	3
Regressions ❌ (secondary)	3.2%	[2.1%, 4.8%]	3
Improvements ✅ (primary)	-2.4%	[-2.9%, -2.0%]	2
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-0.0%	[-2.9%, 3.2%]	5

Cycles

Results (primary -3.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-3.1%	[-3.1%, -3.1%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-3.1%	[-3.1%, -3.1%]	1

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 481.762s -> 496.521s (3.06%)
Artifact size: 397.93 MiB -> 397.97 MiB (0.01%)

[petrochenkov]

Nice.
@craterbot check

[craterbot]

👌 Experiment pr-145108-2 created and queued.
🤖 Automatically detected try build fa4dd1f
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

🚧 Experiment pr-145108-2 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[rust-bors]

☔ The latest upstream changes (presumably #152924) made this pull request unmergeable. Please resolve the merge conflicts.

[craterbot]

🎉 Experiment pr-145108-2 is completed!
📊 161 regressed and 1 fixed (821749 total)
📊 2472 spurious results on the retry-regressed-list.txt, consider a retry¹ if this is a significant amount.
📰 Open the summary report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

Footnotes

1. re-run the experiment with crates=https://crater-reports.s3.amazonaws.com/pr-145108-2/retry-regressed-list.txt ↩

[petrochenkov]

Looks like rust-embed is still a problem.
Could you rebase, squash commits and re-apply the rust-embed hack commit on top of the other changes?
@rustbot author