Use a separate interner type for UniqueTypeId #87867

bjorn3 · 2021-08-08T15:25:03Z

Using symbol::Interner makes it very easy to mixup UniqueTypeId symbols
with the global interner. In fact the Debug implementation of
UniqueTypeId did exactly this.

Using a separate interner type also avoids prefilling the interner with
unused symbols and allow for optimizing the symbol interner for parallel
access without negatively affecting the single threaded module codegen.

rust-highfive · 2021-08-08T15:25:06Z

r? @wesleywiser

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

bjorn3 · 2021-08-08T15:25:16Z

@bors try @rust-timer queue

rust-timer · 2021-08-08T15:25:17Z

Awaiting bors try build completion.

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

bors · 2021-08-08T15:25:23Z

⌛ Trying commit a35943dfb8749ea72b73d4b77eb0a039e0b69424 with merge 476bbb073a2cb29fca275fdb4960f71270623a3c...

bors · 2021-08-08T16:15:38Z

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

rust-timer · 2021-08-08T16:15:40Z

Queued 476bbb073a2cb29fca275fdb4960f71270623a3c with parent e8c25f2, future comparison URL.

bjorn3 · 2021-08-08T16:24:26Z

compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs

+    use rustc_arena::DroplessArena;
+
+    #[derive(Copy, Hash, Eq, PartialEq, Clone)]
+    pub(super) struct UniqueTypeId(u32);


This type used to be a wrapper around Symbol with a Debug derive that used the global interner, but the Symbol would actually be interned by a separate interner.

rust-timer · 2021-08-08T19:15:25Z

Finished benchmarking try commit (476bbb073a2cb29fca275fdb4960f71270623a3c): comparison url.

Summary: This benchmark run did not return any significant changes.

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.

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

bjorn3 · 2021-08-08T20:23:03Z

Up to 0.3% regressions all of which are for opt builds, which shouldn't have debuginfo generation and thus not run the changed code. Maybe it affects cgu partitioning or inlining?

JohnCSimon · 2021-09-13T02:40:19Z

Ping from triage:
@bjorn3 - can you please address the merge conflicts?

Using symbol::Interner makes it very easy to mixup UniqueTypeId symbols with the global interner. In fact the Debug implementation of UniqueTypeId did exactly this. Using a separate interner type also avoids prefilling the interner with unused symbols and allow for optimizing the symbol interner for parallel access without negatively affecting the single threaded module codegen.

bjorn3 · 2021-09-13T12:42:46Z

Rebased

wesleywiser · 2021-09-13T16:33:45Z

@bors r+

bors · 2021-09-13T16:33:46Z

📌 Commit 8c7840e has been approved by wesleywiser

bors · 2021-09-14T01:07:16Z

⌛ Testing commit 8c7840e with merge dd52fb9058f3ddbb2e2f28460169860e7f387176...

rust-log-analyzer · 2021-09-14T02:09:03Z

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

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

[TIMING] Rustc { target: TargetSelection { triple: "x86_64-pc-windows-msvc", file: None }, compiler: Compiler { stage: 1, host: TargetSelection { triple: "x86_64-pc-windows-msvc", file: None } } } -- 0.938
Assembling stage2 compiler (x86_64-pc-windows-msvc)
[TIMING] Sysroot { compiler: Compiler { stage: 2, host: TargetSelection { triple: "x86_64-pc-windows-msvc", file: None } } } -- 0.000
[TIMING] Libdir { compiler: Compiler { stage: 2, host: TargetSelection { triple: "x86_64-pc-windows-msvc", file: None } }, target: TargetSelection { triple: "x86_64-pc-windows-msvc", file: None } } -- 0.000
thread 'main' panicked at 'failed to copy `D:\a\rust\rust\build\x86_64-pc-windows-msvc\stage1-rustc\x86_64-pc-windows-msvc\release\rustc-main.exe` to `D:\a\rust\rust\build\x86_64-pc-windows-msvc\stage2\bin\rustc.exe`: The process cannot access the file because it is being used by another process. (os error 32)', src\bootstrap\lib.rs:1335:17
Build completed unsuccessfully in 0:00:04

bors · 2021-09-14T02:09:47Z

💔 Test failed - checks-actions

ehuss · 2021-09-14T21:31:10Z

@bors retry

#88924

bors · 2021-09-14T21:40:21Z

⌛ Testing commit 8c7840e with merge 9e8b44cb5b43b95c220986e4fbab1fa8fc6c7930...

rust-log-analyzer · 2021-09-14T22:10:08Z

The job dist-i686-msvc failed! Check out the build log: (web) (plain)

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

   Compiling core v0.0.0 (D:\a\rust\rust\library\core)
   Compiling libc v0.2.99
   Compiling memchr v2.4.1
   Compiling std v0.0.0 (D:\a\rust\rust\library\std)
error: linking with `link.exe` failed: exit code: 1104
  |
  = note: "C:\\Program Files (x86)\\Microsoft Visual Studio\\2019\\Enterprise\\VC\\Tools\\MSVC\\14.29.30133\\bin\\HostX64\\x86\\link.exe" "/NOLOGO" "/LARGEADDRESSAWARE" "/SAFESEH" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2-std\\release\\build\\libc-8ef7e2152eb68624\\build_script_build-8ef7e2152eb68624.build_script_build.ffed8922-cgu.0.rcgu.o" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2-std\\release\\build\\libc-8ef7e2152eb68624\\build_script_build-8ef7e2152eb68624.build_script_build.ffed8922-cgu.1.rcgu.o" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2-std\\release\\build\\libc-8ef7e2152eb68624\\build_script_build-8ef7e2152eb68624.build_script_build.ffed8922-cgu.10.rcgu.o" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2-std\\release\\build\\libc-8ef7e2152eb68624\\build_script_build-8ef7e2152eb68624.build_script_build.ffed8922-cgu.11.rcgu.o" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2-std\\release\\build\\libc-8ef7e2152eb68624\\build_script_build-8ef7e2152eb68624.build_script_build.ffed8922-cgu.12.rcgu.o" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2-std\\release\\build\\libc-8ef7e2152eb68624\\build_script_build-8ef7e2152eb68624.build_script_build.ffed8922-cgu.13.rcgu.o" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2-std\\release\\build\\libc-8ef7e2152eb68624\\build_script_build-8ef7e2152eb68624.build_script_build.ffed8922-cgu.14.rcgu.o" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2-std\\release\\build\\libc-8ef7e2152eb68624\\build_script_build-8ef7e2152eb68624.build_script_build.ffed8922-cgu.15.rcgu.o" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2-std\\release\\build\\libc-8ef7e2152eb68624\\build_script_build-8ef7e2152eb68624.build_script_build.ffed8922-cgu.2.rcgu.o" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2-std\\release\\build\\libc-8ef7e2152eb68624\\build_script_build-8ef7e2152eb68624.build_script_build.ffed8922-cgu.3.rcgu.o" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2-std\\release\\build\\libc-8ef7e2152eb68624\\build_script_build-8ef7e2152eb68624.build_script_build.ffed8922-cgu.4.rcgu.o" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2-std\\release\\build\\libc-8ef7e2152eb68624\\build_script_build-8ef7e2152eb68624.build_script_build.ffed8922-cgu.5.rcgu.o" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2-std\\release\\build\\libc-8ef7e2152eb68624\\build_script_build-8ef7e2152eb68624.build_script_build.ffed8922-cgu.6.rcgu.o" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2-std\\release\\build\\libc-8ef7e2152eb68624\\build_script_build-8ef7e2152eb68624.build_script_build.ffed8922-cgu.7.rcgu.o" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2-std\\release\\build\\libc-8ef7e2152eb68624\\build_script_build-8ef7e2152eb68624.build_script_build.ffed8922-cgu.8.rcgu.o" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2-std\\release\\build\\libc-8ef7e2152eb68624\\build_script_build-8ef7e2152eb68624.build_script_build.ffed8922-cgu.9.rcgu.o" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2-std\\release\\build\\libc-8ef7e2152eb68624\\build_script_build-8ef7e2152eb68624.1ufx9vfr6oc6ja33.rcgu.o" "/LIBPATH:D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2-std\\release\\deps" "/LIBPATH:D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage0\\lib\\rustlib\\i686-pc-windows-msvc\\lib" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage0\\lib\\rustlib\\i686-pc-windows-msvc\\lib\\libstd-81cb19e6a508a382.rlib" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage0\\lib\\rustlib\\i686-pc-windows-msvc\\lib\\libpanic_unwind-d68169d5fd63206c.rlib" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage0\\lib\\rustlib\\i686-pc-windows-msvc\\lib\\libstd_detect-f7aff418b211f3b4.rlib" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage0\\lib\\rustlib\\i686-pc-windows-msvc\\lib\\librustc_demangle-bde83a8e075cbb6d.rlib" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage0\\lib\\rustlib\\i686-pc-windows-msvc\\lib\\libhashbrown-d018f29c6b54b3e3.rlib" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage0\\lib\\rustlib\\i686-pc-windows-msvc\\lib\\librustc_std_workspace_alloc-57a11699c9cdcc43.rlib" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage0\\lib\\rustlib\\i686-pc-windows-msvc\\lib\\libunwind-8ea0c0e26bfbf063.rlib" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage0\\lib\\rustlib\\i686-pc-windows-msvc\\lib\\libcfg_if-423bcb240d49abd0.rlib" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage0\\lib\\rustlib\\i686-pc-windows-msvc\\lib\\liblibc-2e26fae3a666b111.rlib" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage0\\lib\\rustlib\\i686-pc-windows-msvc\\lib\\liballoc-dd6f76dfeed99195.rlib" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage0\\lib\\rustlib\\i686-pc-windows-msvc\\lib\\librustc_std_workspace_core-31972101b01c6747.rlib" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage0\\lib\\rustlib\\i686-pc-windows-msvc\\lib\\libcore-4d870864fbc17473.rlib" "D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage0\\lib\\rustlib\\i686-pc-windows-msvc\\lib\\libcompiler_builtins-771055d60d4c83bb.rlib" "kernel32.lib" "ws2_32.lib" "advapi32.lib" "userenv.lib" "kernel32.lib" "libcmt.lib" "/NXCOMPAT" "/LIBPATH:D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage0\\lib\\rustlib\\i686-pc-windows-msvc\\lib" "/OUT:D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage2-std\\release\\build\\libc-8ef7e2152eb68624\\build_script_build-8ef7e2152eb68624.exe" "/OPT:REF,NOICF" "/DEBUG" "/NATVIS:D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage0\\lib\\rustlib\\etc\\intrinsic.natvis" "/NATVIS:D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage0\\lib\\rustlib\\etc\\liballoc.natvis" "/NATVIS:D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage0\\lib\\rustlib\\etc\\libcore.natvis" "/NATVIS:D:\\a\\rust\\rust\\build\\i686-pc-windows-msvc\\stage0\\lib\\rustlib\\etc\\libstd.natvis"
  = note: LINK : fatal error LNK1104: cannot open file 'D:\a\rust\rust\build\i686-pc-windows-msvc\stage2-std\release\build\libc-8ef7e2152eb68624\build_script_build-8ef7e2152eb68624.exe'
          

[RUSTC-TIMING] build_script_build test:false 0.315
error: could not compile `libc` due to previous error

bors · 2021-09-14T22:11:03Z

💔 Test failed - checks-actions

CryZe · 2021-09-15T00:04:07Z

The Windows Github Action Runners are broken atm: #88924

ehuss · 2021-09-15T12:21:24Z

@bors retry

#88924

bors · 2021-09-15T12:34:34Z

⌛ Testing commit 8c7840e with merge 2c7bc5e...

bors · 2021-09-15T15:35:48Z

☀️ Test successful - checks-actions
Approved by: wesleywiser
Pushing 2c7bc5e to master...

rust-timer · 2021-09-15T19:33:00Z

Finished benchmarking commit (2c7bc5e): comparison url.

Summary: This benchmark run did not return any relevant changes.

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

@rustbot label: -perf-regression

…ark-Simulacrum Move the Lock into symbol::Interner This makes it easier to make the symbol interner (near) lock free in case of concurrent accesses in the future. With rust-lang#87867 landed this shouldn't affect performance anymore.

JohnTitor · 2021-10-13T04:26:56Z

@bjorn3 This makes rust-semverver confused (e.g. on https://github.com/rust-lang/rust-semverver/blob/b49417044bd4e9833e218b5f370388d9fe191661/src/changes.rs#L1463-L1464). Since some parts (Interner itself, and Symbol's field and assoc fn) are private, it's hard to have our own Intener I guess, is there any workaround?

bjorn3 · 2021-10-13T13:01:44Z

Nornally I think having your own interner is the best solution. The previous setup made it too easy to mixup between different interners, which will result in incorrect results (for example symbol.as_atr() will use the global interner and not your own interner) and can potentially be a safety issue.

In this specific case I think only test is interned though, so you can use sym::test instead, which is also faster as it is already a symbol and doesn't need a hashmap lookup.

JohnTitor · 2021-10-15T01:24:12Z

Got it, thanks for the pointer!

…ust#88677

rust-highfive assigned wesleywiser Aug 8, 2021

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

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

bjorn3 commented Aug 8, 2021

View reviewed changes

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

bjorn3 force-pushed the unique_type_id_interner branch from a35943d to 8c7840e Compare September 13, 2021 12:42

wesleywiser approved these changes Sep 13, 2021

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 Sep 13, 2021

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 Sep 14, 2021

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 14, 2021

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 Sep 14, 2021

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 15, 2021

bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 15, 2021

bors merged commit 2c7bc5e into rust-lang:master Sep 15, 2021

rustbot added this to the 1.57.0 milestone Sep 15, 2021

bjorn3 deleted the unique_type_id_interner branch September 15, 2021 16:14

bjorn3 mentioned this pull request Sep 15, 2021

Move the Lock into symbol::Interner #88978

Merged

JohnTitor mentioned this pull request Oct 13, 2021

Some rustups rust-lang/rust-semverver#227

Merged

JohnTitor added a commit to JohnTitor/rust-semverver that referenced this pull request Oct 15, 2021

Rustup to rust-lang/rust#84373, rust-lang/rust#87867, and rust-lang/r…

7dbee3c

…ust#88677

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use a separate interner type for UniqueTypeId #87867

Use a separate interner type for UniqueTypeId #87867

bjorn3 commented Aug 8, 2021

rust-highfive commented Aug 8, 2021

bjorn3 commented Aug 8, 2021

rust-timer commented Aug 8, 2021

bors commented Aug 8, 2021

bors commented Aug 8, 2021

rust-timer commented Aug 8, 2021

bjorn3 Aug 8, 2021

rust-timer commented Aug 8, 2021

bjorn3 commented Aug 8, 2021

JohnCSimon commented Sep 13, 2021

bjorn3 commented Sep 13, 2021

wesleywiser commented Sep 13, 2021

bors commented Sep 13, 2021

bors commented Sep 14, 2021

rust-log-analyzer commented Sep 14, 2021

bors commented Sep 14, 2021

ehuss commented Sep 14, 2021

bors commented Sep 14, 2021

rust-log-analyzer commented Sep 14, 2021

bors commented Sep 14, 2021

CryZe commented Sep 15, 2021

ehuss commented Sep 15, 2021

bors commented Sep 15, 2021

bors commented Sep 15, 2021

rust-timer commented Sep 15, 2021

JohnTitor commented Oct 13, 2021 •

edited

Loading

bjorn3 commented Oct 13, 2021

JohnTitor commented Oct 15, 2021

Use a separate interner type for UniqueTypeId #87867

Use a separate interner type for UniqueTypeId #87867

Conversation

bjorn3 commented Aug 8, 2021

rust-highfive commented Aug 8, 2021

bjorn3 commented Aug 8, 2021

rust-timer commented Aug 8, 2021

bors commented Aug 8, 2021

bors commented Aug 8, 2021

rust-timer commented Aug 8, 2021

bjorn3 Aug 8, 2021

Choose a reason for hiding this comment

rust-timer commented Aug 8, 2021

bjorn3 commented Aug 8, 2021

JohnCSimon commented Sep 13, 2021

bjorn3 commented Sep 13, 2021

wesleywiser commented Sep 13, 2021

bors commented Sep 13, 2021

bors commented Sep 14, 2021

rust-log-analyzer commented Sep 14, 2021

bors commented Sep 14, 2021

ehuss commented Sep 14, 2021

bors commented Sep 14, 2021

rust-log-analyzer commented Sep 14, 2021

bors commented Sep 14, 2021

CryZe commented Sep 15, 2021

ehuss commented Sep 15, 2021

bors commented Sep 15, 2021

bors commented Sep 15, 2021

rust-timer commented Sep 15, 2021

JohnTitor commented Oct 13, 2021 • edited Loading

bjorn3 commented Oct 13, 2021

JohnTitor commented Oct 15, 2021

JohnTitor commented Oct 13, 2021 •

edited

Loading