-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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.
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
Allow injecting a profiler runtime into #![no_core]
crates
#133369
Conversation
r? @Nadrieril rustbot has assigned @Nadrieril. Use |
I can't say I'm too familiar with |
This crate doesn't contain any actual Rust code; it's just C/C++ code built and packaged in a Rust-friendly way.
89e2ad0
to
bd7aa4b
Compare
These commits modify the If this was unintentional then you should revert the changes before this PR is merged. |
r? compiler It's possible to write a run-make test that actually runs cargo with |
r? jieyouxu |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this seems reasonable to me, just one nit. You can r=me with the nit.
@rustbot author |
@bors rollup=never (profiler runtime setup modifications) |
Now that the profiler runtime is itself `#![no_core]`, it can be a dependency of other no_core crates, including core.
bd7aa4b
to
6798eca
Compare
…r=jieyouxu Allow injecting a profiler runtime into `#![no_core]` crates An alternative to rust-lang#133300, allowing `-Cinstrument-coverage` to be used with `-Zbuild-std`. The incompatibility between `profiler_builtins` and `#![no_core]` crates appears to have been caused by profiler_builtins depending on core, and therefore conflicting with core (or minicore). But that's a false dependency, because the profiler doesn't contain any actual Rust code. So we can just mark the profiler itself as `#![no_core]`, and remove the incompatibility error. --- For context, the error was originally added by rust-lang#79958.
💔 Test failed - checks-actions |
@bors retry (failed to pay msvc taxes) |
☀️ Test successful - checks-actions |
Finished benchmarking commit (39cb338): comparison URL. Overall result: ❌ regressions - no action needed@rustbot label: -perf-regression Instruction countThis is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.
Max RSS (memory usage)Results (primary 3.6%, secondary -0.0%)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.
CyclesResults (primary 4.2%)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.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 794.092s -> 793.948s (-0.02%) |
An alternative to #133300, allowing
-Cinstrument-coverage
to be used with-Zbuild-std
.The incompatibility between
profiler_builtins
and#![no_core]
crates appears to have been caused by profiler_builtins depending on core, and therefore conflicting with core (or minicore).But that's a false dependency, because the profiler doesn't contain any actual Rust code. So we can just mark the profiler itself as
#![no_core]
, and remove the incompatibility error.For context, the error was originally added by #79958.