Use `Freeze` for `SourceFile` by Zoxc · Pull Request #115418 · rust-lang/rust

Zoxc · 2023-08-31T22:19:15Z

This uses the Freeze type in SourceFile to let accessing external_src and lines be lock-free.

Behavior of add_external_src is changed to set ExternalSourceKind::AbsentErr on a hash mismatch which matches the documentation. ExternalSourceKind::Unneeded was removed as it's unused.

Based on #115401.

rustbot · 2023-08-31T22:19:21Z

r? @TaKO8Ki

(rustbot has picked a reviewer for you, use r? to override)

rustbot · 2023-08-31T22:19:24Z

Some changes occurred in compiler/rustc_codegen_cranelift

cc @bjorn3

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

compiler/rustc_data_structures/src/sync/freeze.rs

bors · 2023-09-02T05:51:24Z

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

Zoxc · 2023-09-07T09:38:01Z

r? @oli-obk

compiler/rustc_span/src/lib.rs

compiler/rustc_data_structures/src/sync/freeze.rs

oli-obk · 2023-09-07T10:32:26Z

compiler/rustc_span/src/lib.rs

+                let lock = self.lines.read();
+                FreezeLock::with(lock.clone(), self.lines.is_frozen())


since there are two occurrences of this now, maybe add a non-self method to FreezeLock for cloning the lock?

oli-obk · 2023-09-07T10:33:33Z

@bors try @rust-timer queue

bors · 2023-09-07T10:33:43Z

⌛ Trying commit f96fe5b with merge dc98c3e...

Use `Freeze` for `SourceFile` This uses the `Freeze` type in `SourceFile` to let accessing `external_src` and `lines` be lock-free. Behavior of `add_external_src` is changed to set `ExternalSourceKind::AbsentErr` on a hash mismatch which matches the documentation. `ExternalSourceKind::Unneeded` was removed as it's unused. Based on rust-lang#115401.

bors · 2023-09-07T11:50:57Z

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

rust-timer · 2023-09-07T15:02:18Z

Finished benchmarking commit (dc98c3e): comparison URL.

Overall result: ✅ improvements - no action needed

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-perf -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

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

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

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

Cycles

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

Binary size

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

Bootstrap: 628.665s -> 629.523s (0.14%)
Artifact size: 318.00 MiB -> 317.97 MiB (-0.01%)

oli-obk · 2023-09-08T16:39:06Z

@bors r+

bors · 2023-09-08T16:39:08Z

📌 Commit c83eba9 has been approved by oli-obk

It is now in the queue for this repository.

bors · 2023-09-08T17:20:27Z

⌛ Testing commit c83eba9 with merge 26f4b72...

bors · 2023-09-08T19:07:10Z

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

rust-timer · 2023-09-08T20:28:42Z

Finished benchmarking commit (26f4b72): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

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

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

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

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

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

Binary size

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

Bootstrap: 629.933s -> 627.243s (-0.43%)
Artifact size: 318.10 MiB -> 318.06 MiB (-0.02%)

Use `Freeze` for `SourceFile` This uses the `Freeze` type in `SourceFile` to let accessing `external_src` and `lines` be lock-free. Behavior of `add_external_src` is changed to set `ExternalSourceKind::AbsentErr` on a hash mismatch which matches the documentation. `ExternalSourceKind::Unneeded` was removed as it's unused. Based on rust-lang#115401.

rustbot assigned TaKO8Ki Aug 31, 2023

This comment has been minimized.

Sign in to view

Zoxc force-pushed the freeze-source branch from 55e9aaf to 317a76d Compare August 31, 2023 22:51

bjorn3 reviewed Sep 1, 2023

View reviewed changes

compiler/rustc_data_structures/src/sync/freeze.rs Outdated Show resolved Hide resolved

Zoxc mentioned this pull request Sep 1, 2023

Add FreezeLock type and use it to store Definitions #115401

Merged

Zoxc force-pushed the freeze-source branch from 317a76d to f96fe5b Compare September 7, 2023 05:21

rustbot assigned oli-obk and unassigned TaKO8Ki Sep 7, 2023

oli-obk reviewed Sep 7, 2023

View reviewed changes

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Sep 7, 2023

Zoxc added 2 commits September 7, 2023 13:04

Use Freeze for SourceFile.external_src

c5996b8

Use Freeze for SourceFile.lines

f49382c

This comment has been minimized.

Sign in to view

Zoxc force-pushed the freeze-source branch from f96fe5b to f49382c Compare September 7, 2023 11:52

rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Sep 7, 2023

Add Freeze::clone

c83eba9

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 Sep 8, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 8, 2023

bors merged commit 26f4b72 into rust-lang:master Sep 8, 2023

rustbot added this to the 1.74.0 milestone Sep 8, 2023

Zoxc deleted the freeze-source branch September 8, 2023 22:52

xFrednet mentioned this pull request Sep 10, 2023

Bump Nightly 2023-10-05 rust-marker/marker#236

Closed

		let lock = self.lines.read();
		FreezeLock::with(lock.clone(), self.lines.is_frozen())

Uh oh!

Conversation

Zoxc commented Aug 31, 2023

Uh oh!

rustbot commented Aug 31, 2023

Uh oh!

rustbot commented Aug 31, 2023

Uh oh!

This comment has been minimized.

Uh oh!

bors commented Sep 2, 2023

Uh oh!

Zoxc commented Sep 7, 2023

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oli-obk commented Sep 7, 2023

Uh oh!

This comment has been minimized.

bors commented Sep 7, 2023

Uh oh!

bors commented Sep 7, 2023

Uh oh!

This comment has been minimized.

rust-timer commented Sep 7, 2023

Overall result: ✅ improvements - no action needed

Uh oh!

oli-obk commented Sep 8, 2023

Uh oh!

bors commented Sep 8, 2023

Uh oh!

bors commented Sep 8, 2023

Uh oh!

bors commented Sep 8, 2023

Uh oh!

rust-timer commented Sep 8, 2023

Overall result: ✅ improvements - no action needed

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants