Optimize switch sources representation and usage #96838

tmiasko · 2022-05-08T14:33:08Z

Avoid constructing switch sources unless necessary - switch sources are used by backward analysis with a custom switch int edge effects, but are otherwise unnecessarily computed.
Use sparse representation of switch sources to avoid quadratic space overhead.

rust-highfive · 2022-05-08T14:33:11Z

r? @cjgillot

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

tmiasko · 2022-05-08T14:33:35Z

@bors try @rust-timer queue

rust-timer · 2022-05-08T14:33:36Z

Awaiting bors try build completion.

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

bors · 2022-05-08T14:33:42Z

⌛ Trying commit 1293d9a22f03f8b5c6c4fd24679b7c06f7cbbdcf with merge 7ffecc61411100715256f3f56f13f5846a4a013b...

bors · 2022-05-08T16:01:18Z

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

rust-timer · 2022-05-08T16:01:20Z

Queued 7ffecc61411100715256f3f56f13f5846a4a013b with parent 8fbd92d, future comparison URL.

rust-timer · 2022-05-08T17:46:28Z

Finished benchmarking commit (7ffecc61411100715256f3f56f13f5846a4a013b): comparison url.

Summary:

Primary benchmarks: 🎉 relevant improvements found
Secondary benchmarks: 🎉 relevant improvements found

	Regressions 😿 (primary)	Regressions 😿 (secondary)	Improvements 🎉 (primary)	Improvements 🎉 (secondary)	All 😿 🎉 (primary)
count¹	0	0	4	6	4
mean²	N/A	N/A	-0.7%	-2.4%	-0.7%
max	N/A	N/A	-1.0%	-3.0%	-1.0%

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 may lead to changes in compiler perf.

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

number of relevant changes ↩
the arithmetic mean of the percent change ↩

Switch sources are used by backward analysis with a custom switch int edge effects, but are otherwise unnecessarily computed. Delay the computation until we know that switch sources are indeed required and avoid the computation otherwise.

to avoid quadratic space overhead

oli-obk · 2022-05-09T06:53:57Z

compiler/rustc_mir_dataflow/src/framework/direction.rs

@@ -322,7 +322,8 @@ where
    fn apply(&mut self, mut apply_edge_effect: impl FnMut(&mut D, SwitchIntTarget)) {
        assert!(!self.effects_applied);

-        let targets = self.values.iter().map(|&value| SwitchIntTarget { value, target: self.bb });
+        let values = &self.body.switch_sources()[self.bb][self.pred];


I don't expect this to affect much, but since we're only interested in self.pred, it could be cheaper to just look at the terminator of basic block with id self.pred and iterate over all targets filtering for ones for self.bb. Caching this map does not seem very useful if "computing" it for specific blocks is just a lookup and an iteration.

Considering that this is the only user of switch_sources, I'm not sure it is carrying its weight.

According to the discussion on the original pull request #95120, switch sources were introduced to avoid quadratic time complexity associated with the implementation strategy you are just suggesting.

I guess if we have a switch with lots of targets... 🤷 added to my todo to do some experiments in the future

oli-obk · 2022-05-09T09:28:10Z

@bors r+ rollup=never

bors · 2022-05-09T09:28:12Z

📌 Commit 2be012a has been approved by oli-obk

bors · 2022-05-09T14:30:08Z

⌛ Testing commit 2be012a with merge 1eaacc30576db615c0add464c71199f4230008e9...

rust-log-analyzer · 2022-05-09T14:57:32Z

The job x86_64-gnu-tools failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

.......... (60/64)
...       (64/64)


/checkout/src/test/rustdoc-gui/search-filter.goml search-filter... FAILED
[ERROR] (line 6) TimeoutError: waiting for selector "#titles" failed: timeout 30000ms exceeded: for command `wait-for: "#titles"`
Build completed unsuccessfully in 0:00:41

bors · 2022-05-09T14:58:17Z

💔 Test failed - checks-actions

oli-obk · 2022-05-09T14:58:22Z

@bors retry

bors · 2022-05-09T22:15:33Z

⌛ Testing commit 2be012a with merge cb39073...

bors · 2022-05-10T00:40:28Z

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing cb39073 to master...

rust-timer · 2022-05-10T02:00:22Z

Finished benchmarking commit (cb39073): comparison url.

Summary:

Primary benchmarks: 🎉 relevant improvements found
Secondary benchmarks: 🎉 relevant improvements found

	Regressions 😿 (primary)	Regressions 😿 (secondary)	Improvements 🎉 (primary)	Improvements 🎉 (secondary)	All 😿 🎉 (primary)
count¹	0	0	4	7	4
mean²	N/A	N/A	-0.7%	-2.0%	-0.7%
max	N/A	N/A	-1.0%	-2.6%	-1.0%

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

@rustbot label: -perf-regression

number of relevant changes ↩
the arithmetic mean of the percent change ↩

rust-highfive assigned cjgillot May 8, 2022

rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label May 8, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 8, 2022

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 8, 2022

tmiasko mentioned this pull request May 8, 2022

Fix Dest Prop #96451

Merged

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 8, 2022

tmiasko added 2 commits May 8, 2022 23:14

Use sparse representation of switch sources

2be012a

to avoid quadratic space overhead

tmiasko force-pushed the lazy-switch-sources branch from 1293d9a to 2be012a Compare May 8, 2022 21:52

tmiasko changed the title ~~Avoid constructing switch sources unless strictly necessary~~ Optimize switch sources representation and usage May 8, 2022

oli-obk reviewed May 9, 2022

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 9, 2022

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels May 9, 2022

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 9, 2022

bors added the merged-by-bors This PR was explicitly merged by bors. label May 10, 2022

bors merged commit cb39073 into rust-lang:master May 10, 2022

rustbot added this to the 1.62.0 milestone May 10, 2022

JakobDegen mentioned this pull request May 10, 2022

Rollup of 5 pull requests #95382

Merged

tmiasko deleted the lazy-switch-sources branch May 11, 2022 07:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimize switch sources representation and usage #96838

Optimize switch sources representation and usage #96838

tmiasko commented May 8, 2022 •

edited

Loading

rust-highfive commented May 8, 2022

tmiasko commented May 8, 2022

rust-timer commented May 8, 2022

bors commented May 8, 2022

bors commented May 8, 2022

rust-timer commented May 8, 2022

rust-timer commented May 8, 2022

oli-obk May 9, 2022

oli-obk May 9, 2022

tmiasko May 9, 2022

oli-obk May 9, 2022

oli-obk commented May 9, 2022

bors commented May 9, 2022

bors commented May 9, 2022

rust-log-analyzer commented May 9, 2022

bors commented May 9, 2022

oli-obk commented May 9, 2022

bors commented May 9, 2022

bors commented May 10, 2022

rust-timer commented May 10, 2022

Optimize switch sources representation and usage #96838

Optimize switch sources representation and usage #96838

Conversation

tmiasko commented May 8, 2022 • edited Loading

rust-highfive commented May 8, 2022

tmiasko commented May 8, 2022

rust-timer commented May 8, 2022

bors commented May 8, 2022

bors commented May 8, 2022

rust-timer commented May 8, 2022

rust-timer commented May 8, 2022

Footnotes

oli-obk May 9, 2022

Choose a reason for hiding this comment

oli-obk May 9, 2022

Choose a reason for hiding this comment

tmiasko May 9, 2022

Choose a reason for hiding this comment

oli-obk May 9, 2022

Choose a reason for hiding this comment

oli-obk commented May 9, 2022

bors commented May 9, 2022

bors commented May 9, 2022

rust-log-analyzer commented May 9, 2022

bors commented May 9, 2022

oli-obk commented May 9, 2022

bors commented May 9, 2022

bors commented May 10, 2022

rust-timer commented May 10, 2022

Footnotes

tmiasko commented May 8, 2022 •

edited

Loading