Skip to content

Optimize position adjustments #97386

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
May 27, 2022
Merged

Optimize position adjustments #97386

merged 2 commits into from
May 27, 2022

Conversation

nnethercote
Copy link
Contributor

A small improvement.

r? @bjorn3

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label May 25, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 25, 2022
@nnethercote
Copy link
Contributor Author

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

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

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 25, 2022
@bors
Copy link
Collaborator

bors commented May 25, 2022

⌛ Trying commit dcce37d3d8c026a1cc1b3450c11d0739bee0f483 with merge e2290d2b5861a40c28cad29dab6029005e3ec044...

@nnethercote
Copy link
Contributor Author

I'll fix the breakage with some wrapping_add calls tomorrow.

@bors
Copy link
Collaborator

bors commented May 25, 2022

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

@rust-timer
Copy link
Collaborator

Queued e2290d2b5861a40c28cad29dab6029005e3ec044 with parent 9fadabc, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (e2290d2b5861a40c28cad29dab6029005e3ec044): comparison url.

Instruction count

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

Max RSS (memory usage)

Results
  • Primary benchmarks: no relevant changes found
  • Secondary benchmarks: 🎉 relevant improvements found
mean1 max count2
Regressions 😿
(primary)		 N/A N/A 0
Regressions 😿
(secondary)		 N/A N/A 0
Improvements 🎉
(primary)		 N/A N/A 0
Improvements 🎉
(secondary)		 -1.7% -2.4% 4
All 😿🎉 (primary) N/A N/A 0

Cycles

Results
  • Primary benchmarks: 🎉 relevant improvements found
  • Secondary benchmarks: 🎉 relevant improvement found
mean1 max count2
Regressions 😿
(primary)		 N/A N/A 0
Regressions 😿
(secondary)		 N/A N/A 0
Improvements 🎉
(primary)		 -2.4% -3.0% 2
Improvements 🎉
(secondary)		 -3.2% -3.2% 1
All 😿🎉 (primary) -2.4% -3.0% 2

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

Footnotes

  1. the arithmetic mean of the percent change ↩ ↩2

  2. number of relevant changes ↩ ↩2

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 25, 2022
@bjorn3
Copy link
Member

bjorn3 commented May 25, 2022

CI fails with an ICE

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (e2290d2b5861a40c28cad29dab6029005e3ec044): comparison url.

Instruction count

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

Max RSS (memory usage)

Results
  • Primary benchmarks: no relevant changes found
  • Secondary benchmarks: 🎉 relevant improvements found
mean1 max count2
Regressions 😿
(primary)		 N/A N/A 0
Regressions 😿
(secondary)		 N/A N/A 0
Improvements 🎉
(primary)		 N/A N/A 0
Improvements 🎉
(secondary)		 -1.7% -2.4% 4
All 😿🎉 (primary) N/A N/A 0

Cycles

Results
  • Primary benchmarks: 🎉 relevant improvements found
  • Secondary benchmarks: 🎉 relevant improvement found
mean1 max count2
Regressions 😿
(primary)		 N/A N/A 0
Regressions 😿
(secondary)		 N/A N/A 0
Improvements 🎉
(primary)		 -2.4% -3.0% 2
Improvements 🎉
(secondary)		 -3.2% -3.2% 1
All 😿🎉 (primary) -2.4% -3.0% 2

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

Footnotes

  1. the arithmetic mean of the percent change ↩ ↩2

  2. number of relevant changes ↩ ↩2

@nnethercote
Augment a comment.
3e810c6
@nnethercote
Avoid adjusting file positions twice.
2b91c40 
`imported_source_files` adjusts lots of file positions, and then calls
`new_imported_source_file`, which then adjust them all again. This
commit combines the two adjustments into one, for a small perf win.
@nnethercote nnethercote force-pushed the optimize-pos-adjustments branch from dcce37d to 2b91c40 Compare May 25, 2022 23:13
@nnethercote
Copy link
Contributor Author

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

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

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 26, 2022
@bors
Copy link
Collaborator

bors commented May 26, 2022

⌛ Trying commit 2b91c40 with merge 6dafb2bd191eef16ed5fbd67a1430f0d9918cd1f...

@bors
Copy link
Collaborator

bors commented May 26, 2022

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

@rust-timer
Copy link
Collaborator

Queued 6dafb2bd191eef16ed5fbd67a1430f0d9918cd1f with parent 99c4758, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (6dafb2bd191eef16ed5fbd67a1430f0d9918cd1f): comparison url.

Instruction count

  • Primary benchmarks: 🎉 relevant improvement found
  • Secondary benchmarks: 🎉 relevant improvements found
mean1 max count2
Regressions 😿
(primary)		 N/A N/A 0
Regressions 😿
(secondary)		 N/A N/A 0
Improvements 🎉
(primary)		 -0.3% -0.3% 1
Improvements 🎉
(secondary)		 -0.2% -0.3% 10
All 😿🎉 (primary) -0.3% -0.3% 1

Max RSS (memory usage)

Results
  • Primary benchmarks: mixed results
  • Secondary benchmarks: 🎉 relevant improvement found
mean1 max count2
Regressions 😿
(primary)		 3.7% 3.7% 1
Regressions 😿
(secondary)		 N/A N/A 0
Improvements 🎉
(primary)		 -3.3% -6.3% 3
Improvements 🎉
(secondary)		 -3.0% -3.0% 1
All 😿🎉 (primary) -1.6% -6.3% 4

Cycles

Results
  • Primary benchmarks: mixed results
  • Secondary benchmarks: 🎉 relevant improvements found
mean1 max count2
Regressions 😿
(primary)		 2.3% 2.3% 1
Regressions 😿
(secondary)		 N/A N/A 0
Improvements 🎉
(primary)		 -2.5% -2.5% 1
Improvements 🎉
(secondary)		 -3.8% -4.2% 2
All 😿🎉 (primary) -0.1% -2.5% 2

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

Footnotes

  1. the arithmetic mean of the percent change ↩ ↩2 ↩3

  2. number of relevant changes ↩ ↩2 ↩3

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label May 26, 2022
@bjorn3
Copy link
Member

bjorn3 commented May 26, 2022

@bors r+

@bors
Copy link
Collaborator

bors commented May 26, 2022

📌 Commit 2b91c40 has been approved by bjorn3

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

bors commented May 26, 2022

⌛ Testing commit 2b91c40 with merge 7b889924e95a6ad637a5f8a082531bea0c31c449...

@bors
Copy link
Collaborator

bors commented May 26, 2022

💔 Test failed - checks-actions

@bors 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 26, 2022
@rust-log-analyzer
Copy link
Collaborator

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

Click to see the possible cause of the failure (guessed by this bot) 
error: make failed
status: exit code: 2
command: "make"
--- stdout -------------------------------
make[1]: Entering directory '/d/a/rust/rust/src/test/run-make-fulldeps/stdin-non-utf8'
cp non-utf8 /d/a/rust/rust/build/x86_64-pc-windows-gnu/test/run-make-fulldeps/stdin-non-utf8/stdin-non-utf8/non-utf.rs
cat /d/a/rust/rust/build/x86_64-pc-windows-gnu/test/run-make-fulldeps/stdin-non-utf8/stdin-non-utf8/non-utf.rs | PATH="/d/a/rust/rust/build/x86_64-pc-windows-gnu/test/run-make-fulldeps/stdin-non-utf8/stdin-non-utf8:D:\a\rust\rust\build\x86_64-pc-windows-gnu\stage2\bin:/d/a/rust/rust/build/x86_64-pc-windows-gnu/stage0-bootstrap-tools/x86_64-pc-windows-gnu/release/deps:/d/a/rust/rust/build/x86_64-pc-windows-gnu/stage0/bin:/d/a/rust/rust/ninja:/d/a/rust/rust/mingw64/bin:/c/hostedtoolcache/windows/Python/3.10.4/x64/Scripts:/c/hostedtoolcache/windows/Python/3.10.4/x64:/usr/bin:/d/a/rust/rust/sccache:/c/Program Files/MongoDB/Server/5.0/bin:/c/aliyun-cli:/c/vcpkg:/c/cf-cli:/c/Program Files (x86)/NSIS:/c/tools/zstd:/c/Program Files/Mercurial:/c/hostedtoolcache/windows/stack/2.7.5/x64:/c/cabal/bin:/c/ghcup/bin:/c/tools/ghc-9.2.2/bin:/c/Program Files/dotnet:/c/mysql/bin:/c/Program Files/R/R-4.2.0/bin/x64:/c/SeleniumWebDrivers/GeckoDriver:/c/Program Files (x86)/sbt/bin:/c/Program Files (x86)/GitHub CLI:/c/Program Files/Git/bin:/c/Program Files (x86)/pipx_bin:/c/hostedtoolcache/windows/go/1.17.10/x64/bin:/c/hostedtoolcache/windows/Python/3.7.9/x64/Scripts:/c/hostedtoolcache/windows/Python/3.7.9/x64:/c/hostedtoolcache/windows/Ruby/2.5.9/x64/bin:/c/tools/kotlinc/bin:/c/hostedtoolcache/windows/Java_Temurin-Hotspot_jdk/8.0.332-9/x64/bin:/c/npm/prefix:/c/Program Files (x86)/Microsoft SDKs/Azure/CLI2/wbin:/c/ProgramData/kind:/c/Program Files/Eclipse Foundation/jdk-8.0.302.8-hotspot/bin:/c/Windows/system32:/c/Windows:/c/Windows/System32/Wbem:/c/Windows/System32/WindowsPowerShell/v1.0:/c/Windows/System32/OpenSSH:/c/ProgramData/Chocolatey/bin:/c/Program Files/Docker:/c/Program Files/PowerShell/7:/c/Program Files/Microsoft/Web Platform Installer:/c/Program Files/dotnet:/c/Program Files/Microsoft SQL Server/130/Tools/Binn:/c/Program Files/Microsoft SQL Server/Client SDK/ODBC/170/Tools/Binn:/c/Program Files (x86)/Windows Kits/10/Windows Performance Toolkit:/c/Program Files (x86)/Microsoft SQL Server/110/DTS/Binn:/c/Program Files (x86)/Microsoft SQL Server/120/DTS/Binn:/c/Program Files (x86)/Microsoft SQL Server/130/DTS/Binn:/c/Program Files (x86)/Microsoft SQL Server/140/DTS/Binn:/c/Program Files (x86)/Microsoft SQL Server/150/DTS/Binn:/c/Program Files/nodejs:/c/Program Files/OpenSSL/bin:/c/Strawberry/c/bin:/c/Strawberry/perl/site/bin:/c/Strawberry/perl/bin:/c/ProgramData/chocolatey/lib/pulumi/tools/Pulumi/bin:/c/Program Files/TortoiseSVN/bin:/c/Program Files/CMake/bin:/c/ProgramData/chocolatey/lib/maven/apache-maven-3.8.5/bin:/c/Program Files/Microsoft Service Fabric/bin/Fabric/Fabric.Code:/c/Program Files/Microsoft SDKs/Service Fabric/Tools/ServiceFabricLocalClusterManager:/c/Program Files/Git/cmd:/c/Program Files/Git/mingw64/bin:/c/Program Files/Git/usr/bin:/c/Program Files/GitHub CLI:/c/tools/php:/c/Program Files (x86)/sbt/bin:/c/SeleniumWebDrivers/ChromeDriver:/c/SeleniumWebDrivers/EdgeDriver:/c/Program Files/Amazon/AWSCLIV2:/c/Program Files/Amazon/SessionManagerPlugin/bin:/c/Program Files/Amazon/AWSSAMCLI/bin:/c/Program Files (x86)/Google/Cloud SDK/google-cloud-sdk/bin:/c/Program Files (x86)/Microsoft BizTalk Server:/c/Program Files/LLVM/bin:/c/Users/runneradmin/.dotnet/tools:/c/Users/runneradmin/.cargo/bin:/c/Users/runneradmin/AppData/Local/Microsoft/WindowsApps" 'D:\a\rust\rust\build\x86_64-pc-windows-gnu\stage2\bin\rustc.exe' --out-dir /d/a/rust/rust/build/x86_64-pc-windows-gnu/test/run-make-fulldeps/stdin-non-utf8/stdin-non-utf8 -L /d/a/rust/rust/build/x86_64-pc-windows-gnu/test/run-make-fulldeps/stdin-non-utf8/stdin-non-utf8  - 2>&1 \
 | "D:\a\rust\rust/src/etc/cat-and-grep.sh" "error: couldn't read from stdin, as it did not contain valid UTF-8"
[[[ begin stdout ]]]
I/O error: operation failed to complete synchronously

[[[ end stdout ]]]
Error: cannot match: error: couldn't read from stdin, as it did not contain valid UTF-8
make[1]: Leaving directory '/d/a/rust/rust/src/test/run-make-fulldeps/stdin-non-utf8'
--- stderr -------------------------------
--- stderr -------------------------------
make[1]: *** [Makefile:5: all] Error 1



failures:
failures:
    [run-make] src/test\run-make-fulldeps\stdin-non-utf8

Some tests failed in compiletest suite=run-make-fulldeps mode=run-make host=x86_64-pc-windows-gnu target=x86_64-pc-windows-gnu
test result: FAILED. 192 passed; 1 failed; 37 ignored; 0 measured; 0 filtered out; finished in 81.43s

Build completed unsuccessfully in 1:24:11
make: *** [Makefile:78: ci-mingw-subset-1] Error 1

@nnethercote
Copy link
Contributor Author

@bors retry

@bors 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 26, 2022
@bors
Copy link
Collaborator

bors commented May 26, 2022

⌛ Testing commit 2b91c40 with merge b2c9872...

@bors
Copy link
Collaborator

bors commented May 27, 2022

☀️ Test successful - checks-actions
Approved by: bjorn3
Pushing b2c9872 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label May 27, 2022
@bors bors merged commit b2c9872 into rust-lang:master May 27, 2022
@rustbot rustbot added this to the 1.63.0 milestone May 27, 2022
@bors bors mentioned this pull request May 27, 2022
Merged
@nnethercote nnethercote deleted the optimize-pos-adjustments branch May 27, 2022 02:38
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (b2c9872): comparison url.

Instruction count

  • Primary benchmarks: no relevant changes found
  • Secondary benchmarks: 😿 relevant regression found
mean1 max count2
Regressions 😿
(primary)		 N/A N/A 0
Regressions 😿
(secondary)		 0.7% 0.7% 1
Improvements 🎉
(primary)		 N/A N/A 0
Improvements 🎉
(secondary)		 N/A N/A 0
All 😿🎉 (primary) N/A N/A 0

Max RSS (memory usage)

Results
  • Primary benchmarks: 😿 relevant regressions found
  • Secondary benchmarks: 😿 relevant regression found
mean1 max count2
Regressions 😿
(primary)		 0.7% 0.8% 2
Regressions 😿
(secondary)		 1.6% 1.6% 1
Improvements 🎉
(primary)		 N/A N/A 0
Improvements 🎉
(secondary)		 N/A N/A 0
All 😿🎉 (primary) 0.7% 0.8% 2

Cycles

Results
  • Primary benchmarks: no relevant changes found
  • Secondary benchmarks: mixed results
mean1 max count2
Regressions 😿
(primary)		 N/A N/A 0
Regressions 😿
(secondary)		 3.2% 4.3% 2
Improvements 🎉
(primary)		 N/A N/A 0
Improvements 🎉
(secondary)		 -2.5% -2.5% 1
All 😿🎉 (primary) N/A N/A 0

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

@rustbot label: -perf-regression

Footnotes

  1. the arithmetic mean of the percent change ↩ ↩2 ↩3

  2. number of relevant changes ↩ ↩2 ↩3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@bjorn3 bjorn3

Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.63.0
Development

Successfully merging this pull request may close these issues.
7 participants
@nnethercote @rust-timer @bors @bjorn3 @rust-log-analyzer @rust-highfive @rustbot