Skip to content

Conversation

Zoxc
Copy link
Contributor

@Zoxc Zoxc commented Feb 1, 2023

That adds a rustc_shared library which contains all the rustc library crates in a single dylib. It takes over this role from rustc_driver. This is done so that rustc_driver can be compiled in parallel with other crates. rustc_shared is intentionally left empty so it only does linking.

An alternative could be to move the code currently in rustc_driver into a new crate to avoid changing the name of the distributed library.

@rustbot
Copy link
Collaborator

rustbot commented Feb 1, 2023

r? @Nilstrieb

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

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 1, 2023
@Noratrieb
Copy link
Member

Do you have some benchmarks from the impact this has on rustc full and incremental compile times?

@Zoxc
Copy link
Contributor Author

Zoxc commented Feb 1, 2023

The improvement seems to be proportional to the time it takes to compile code in rustc_driver which is 6.2 seconds on my system.

@rust-log-analyzer

This comment has been minimized.

@jyn514
Copy link
Member

jyn514 commented Feb 2, 2023

Let's keep the crate named rustc_driver to avoid unnecessary breaking changes, please. The new crate can be called rustc_driver_impl or something like that.

@rustbot rustbot added the A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic label Feb 2, 2023
@rustbot
Copy link
Collaborator

rustbot commented Feb 2, 2023

rustc_error_messages was changed

cc @davidtwco, @compiler-errors, @JohnTitor, @estebank, @TaKO8Ki

@Zoxc Zoxc changed the title Add a rustc_shared library which contains all the rustc library crates in a single dylib Move code in rustc_driver out to a new rustc_driver_impl crate to allow pipelining Feb 2, 2023
@Zoxc
Copy link
Contributor Author

Zoxc commented Feb 2, 2023

Yeah, that probably is the simplest option. Skip the first commit when reviewing.

@WalterSmuts
Copy link

Hi Folks, just had a question relating to this and not sure where else to ask.

How does rustc avoid linking this as a dynamic library? I see rustc doens't link it dynamically:

[walter@cuddles rust]$ ldd $(which rustc)
	linux-vdso.so.1 (0x00007fff38d6a000)
	libgcc_s.so.1 => /usr/lib/libgcc_s.so.1 (0x00007f7606ed6000)
	librt.so.1 => /usr/lib/librt.so.1 (0x00007f7606ed1000)
	libpthread.so.0 => /usr/lib/libpthread.so.0 (0x00007f7606ecc000)
	libm.so.6 => /usr/lib/libm.so.6 (0x00007f7606de4000)
	libdl.so.2 => /usr/lib/libdl.so.2 (0x00007f7606ddf000)
	libc.so.6 => /usr/lib/libc.so.6 (0x00007f7605e19000)
	/lib64/ld-linux-x86-64.so.2 => /usr/lib64/ld-linux-x86-64.so.2 (0x00007f7606f25000)

But when I try linking to rustc_driver from another project:

#![feature(rustc_private)]
extern crate rustc_driver;

fn main() {
    println!("Hello, world!");
}

I get the following binary:

[walter@cuddles linking-example]$ ldd target/release/linking-example
	linux-vdso.so.1 (0x00007ffecbdae000)
	libstd-62c4894b82797b30.so => not found                     <========# Looking at this entry
	libc.so.6 => /usr/lib/libc.so.6 (0x00007f53b4da1000)
	/lib64/ld-linux-x86-64.so.2 => /usr/lib64/ld-linux-x86-64.so.2 (0x00007f53b4fbc000)

Reason I care is because having a dynamically linked libstd in a project makes it difficult distributing binaries for the project. E.g. creusot

@bjorn3
Copy link
Member

bjorn3 commented Feb 4, 2023

I see rustc doens't link it dynamically

You are likely looking at the rustup wrapper, not rustc itself. Try ldd $(rustup which rustc):

$ ldd $(rustup which rustc)
        linux-vdso.so.1 (0x00007ffff1dda000)
        librustc_driver-2e8cbd5b30733393.so => /home/bjorn/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/../lib/librustc_driver-2e8cbd5b30733393.so (0x00007fa2d6766000)
        libstd-2a15b3cd0948397b.so => /home/bjorn/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/../lib/libstd-2a15b3cd0948397b.so (0x00007fa2d65cc000)
        libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007fa2d659d000)
        librt.so.1 => /lib/x86_64-linux-gnu/librt.so.1 (0x00007fa2d6593000)
        libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007fa2d6571000)
        libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007fa2d639c000)
        libLLVM-15-rust-1.67.0-stable.so => /home/bjorn/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/../lib/../lib/libLLVM-15-rust-1.67.0-stable.so (0x00007fa2cf6b4000)
        libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007fa2cf69a000)
        libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007fa2cf556000)
        /lib64/ld-linux-x86-64.so.2 (0x00007fa2dacd8000)
        libz.so.1 => /lib/x86_64-linux-gnu/libz.so.1 (0x00007fa2cf539000)

Rustc is always dynamically linked. While it would technically be possible to make a statically linked version if you modify rustc_driver/Cargo.toml to omit crate-type = ["dylib"], doing so will make it impossible to load codegen backends other than the LLVM backend linked into rustc itself.

@WalterSmuts
Copy link

Thank you. That makes sense.

@jyn514
Copy link
Member

jyn514 commented Feb 4, 2023

@bors r+ rollup=never (want to see how this affects bootstrap times)

@bors
Copy link
Collaborator

bors commented Feb 4, 2023

📌 Commit 2b8f892 has been approved by jyn514

It is now in the queue for this repository.

@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 Feb 4, 2023
@bors
Copy link
Collaborator

bors commented Feb 4, 2023

⌛ Testing commit 2b8f892 with merge 3de7d7f...

@bors
Copy link
Collaborator

bors commented Feb 4, 2023

☀️ Test successful - checks-actions
Approved by: jyn514
Pushing 3de7d7f to master...

1 similar comment
@bors
Copy link
Collaborator

bors commented Feb 4, 2023

☀️ Test successful - checks-actions
Approved by: jyn514
Pushing 3de7d7f to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Feb 4, 2023
@bors bors merged commit 3de7d7f into rust-lang:master Feb 4, 2023
@rustbot rustbot added this to the 1.69.0 milestone Feb 4, 2023
@rust-timer
Copy link
Collaborator

Finished benchmarking commit (3de7d7f): comparison URL.

Overall result: ❌ regressions - 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.4% [0.4%, 0.4%] 2
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
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)
1.0% [1.0%, 1.0%] 1
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Cycles

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

@Zoxc Zoxc deleted the rustc-shared branch February 5, 2023 10:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic 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
Development

Successfully merging this pull request may close these issues.

9 participants