-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Overhaul ThinVec
usage
#100666
Overhaul ThinVec
usage
#100666
Conversation
Some changes occurred in src/tools/rustfmt cc @rust-lang/rustfmt Some changes occurred in src/librustdoc/clean/types.rs cc @camelid |
@bors try @rust-timer queue |
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
⌛ Trying commit 26873fedff6b77b9233a5437b67da4bfd0756e26 with merge 2f4580eb83980655419fbe5c0ddaefdaf513d628... |
This comment has been minimized.
This comment has been minimized.
SIGILL?! This PR introduces the use of the external Uncontrolled crashes are incredibly rare in rustc in my experience, and |
Hmm. Because I was suspicious about |
☀️ Try build successful - checks-actions |
Queued 2f4580eb83980655419fbe5c0ddaefdaf513d628 with parent 86c6ebe, future comparison URL. |
The stdlib has some debug assertions (see #92686) which will It's like that instead of a panic for code size reasons (and a panic might cause worse stuff when code didn't expect I don't know for sure if it's that, but it might be. See: |
Yep, it looks to be a debug assertion being tripped. (Slice index out of bounds with get_unchecked) Compiling std v0.0.0 (/home/jess/src/rust/library/std)
thread 'rustc' panicked at 'oh no!!', /home/jess/src/rust/library/core/src/slice/index.rs:225:13
stack backtrace:
0: rust_begin_unwind
at ./library/std/src/panicking.rs:584:5
1: core::panicking::panic_fmt
at ./library/core/src/panicking.rs:142:14
2: <alloc::vec::Vec<rustc_ast::ptr::P<rustc_ast::ast::Expr>> as rustc_data_structures::map_in_place::MapInPlace<rustc_ast::ptr::P<rustc_ast::ast::Expr>>>::flat_map_in_place::<rustc_ast::mut_visit::visit_exprs<rustc_expand::expand::InvocationCollector>::{closure#0}, core::option::Option<rustc_ast::ptr::P<rustc_ast::ast::Expr>>>
3: <rustc_ast::ast::Crate as rustc_expand::expand::InvocationCollectorNode>::noop_visit::<rustc_expand::expand::InvocationCollector>
at ./compiler/rustc_expand/src/expand.rs:1371:9
4: <rustc_expand::expand::InvocationCollector>::visit_node::<rustc_ast::ast::Crate>::{closure#2}
at ./compiler/rustc_expand/src/expand.rs:1737:61
5: <rustc_expand::expand::InvocationCollector>::visit_node::<rustc_ast::ast::Crate>
at ./compiler/rustc_expand/src/expand.rs:1737:21
6: <rustc_expand::expand::MacroExpander>::collect_invocations
at ./compiler/rustc_expand/src/expand.rs:557:13
7: <rustc_expand::expand::MacroExpander>::fully_expand_fragment
at ./compiler/rustc_expand/src/expand.rs:392:13
8: <rustc_expand::expand::MacroExpander>::expand_crate
at ./compiler/rustc_expand/src/expand.rs:379:21
9: rustc_interface::passes::configure_and_expand::{closure#1}::{closure#1}
at ./compiler/rustc_interface/src/passes.rs:336:50
10: <rustc_data_structures::profiling::VerboseTimingGuard>::run::<rustc_ast::ast::Crate, rustc_interface::passes::configure_and_expand::{closure#1}::{closure#1}>
at ./compiler/rustc_data_structures/src/profiling.rs:739:9
11: <rustc_session::session::Session>::time::<rustc_ast::ast::Crate, rustc_interface::passes::configure_and_expand::{closure#1}::{closure#1}>
at ./compiler/rustc_session/src/utils.rs:10:9
12: rustc_interface::passes::configure_and_expand::{closure#1}
at ./compiler/rustc_interface/src/passes.rs:336:21
13: <rustc_data_structures::profiling::VerboseTimingGuard>::run::<core::result::Result<rustc_ast::ast::Crate, rustc_errors::ErrorGuaranteed>, rustc_interface::passes::configure_and_expand::{closure#1}>
at ./compiler/rustc_data_structures/src/profiling.rs:739:9
14: <rustc_session::session::Session>::time::<core::result::Result<rustc_ast::ast::Crate, rustc_errors::ErrorGuaranteed>, rustc_interface::passes::configure_and_expand::{closure#1}>
at ./compiler/rustc_session/src/utils.rs:10:9
15: rustc_interface::passes::configure_and_expand
at ./compiler/rustc_interface/src/passes.rs:288:13
16: <rustc_interface::queries::Queries>::expansion::{closure#0}::{closure#0}
at ./compiler/rustc_interface/src/queries.rs:181:17
17: <rustc_interface::passes::boxed_resolver::BoxedResolver>::access::<<rustc_interface::queries::Queries>::expansion::{closure#0}::{closure#0}, core::result::Result<rustc_ast::ast::Crate, rustc_errors::ErrorGuaranteed>>
at ./compiler/rustc_interface/src/passes.rs:132:13
18: <rustc_interface::queries::Queries>::expansion::{closure#0}
at ./compiler/rustc_interface/src/queries.rs:180:25
19: <rustc_interface::queries::Query<(alloc::rc::Rc<rustc_ast::ast::Crate>, alloc::rc::Rc<core::cell::RefCell<rustc_interface::passes::boxed_resolver::BoxedResolver>>, alloc::rc::Rc<rustc_lint::context::LintStore>)>>::compute::<<rustc_interface::queries::Queries>::expansion::{closure#0}>
at ./compiler/rustc_interface/src/queries.rs:37:28
20: <rustc_interface::queries::Queries>::expansion
at ./compiler/rustc_interface/src/queries.rs:169:9
21: rustc_driver::run_compiler::{closure#1}::{closure#2}
at ./compiler/rustc_driver/src/lib.rs:358:13
22: <rustc_interface::interface::Compiler>::enter::<rustc_driver::run_compiler::{closure#1}::{closure#2}, core::result::Result<core::option::Option<rustc_interface::queries::Linker>, rustc_errors::ErrorGuaranteed>>
at ./compiler/rustc_interface/src/queries.rs:381:19
23: rustc_driver::run_compiler::{closure#1}
at ./compiler/rustc_driver/src/lib.rs:309:22
24: rustc_interface::interface::create_compiler_and_run::<core::result::Result<(), rustc_errors::ErrorGuaranteed>, rustc_driver::run_compiler::{closure#1}>::{closure#1}
at ./compiler/rustc_interface/src/interface.rs:323:13
25: rustc_span::with_source_map::<core::result::Result<(), rustc_errors::ErrorGuaranteed>, rustc_interface::interface::create_compiler_and_run<core::result::Result<(), rustc_errors::ErrorGuaranteed>, rustc_driver::run_compiler::{closure#1}>::{closure#1}>
at ./compiler/rustc_span/src/lib.rs:986:5
26: rustc_interface::interface::create_compiler_and_run::<core::result::Result<(), rustc_errors::ErrorGuaranteed>, rustc_driver::run_compiler::{closure#1}>
at ./compiler/rustc_interface/src/interface.rs:317:5
27: rustc_interface::interface::run_compiler::<core::result::Result<(), rustc_errors::ErrorGuaranteed>, rustc_driver::run_compiler::{closure#1}>::{closure#0}
at ./compiler/rustc_interface/src/interface.rs:339:12
28: <scoped_tls::ScopedKey<rustc_span::SessionGlobals>>::set::<rustc_interface::interface::run_compiler<core::result::Result<(), rustc_errors::ErrorGuaranteed>, rustc_driver::run_compiler::{closure#1}>::{closure#0}, core::result::Result<(), rustc_errors::ErrorGuaranteed>>
at /home/jess/.cargo/registry/src/github.com-1ecc6299db9ec823/scoped-tls-1.0.0/src/lib.rs:137:9
29: rustc_span::create_session_globals_then::<core::result::Result<(), rustc_errors::ErrorGuaranteed>, rustc_interface::interface::run_compiler<core::result::Result<(), rustc_errors::ErrorGuaranteed>, rustc_driver::run_compiler::{closure#1}>::{closure#0}>
at ./compiler/rustc_span/src/lib.rs:112:5
30: rustc_interface::util::run_in_thread_pool_with_globals::<rustc_interface::interface::run_compiler<core::result::Result<(), rustc_errors::ErrorGuaranteed>, rustc_driver::run_compiler::{closure#1}>::{closure#0}, core::result::Result<(), rustc_errors::ErrorGuaranteed>>::{closure#0}
at ./compiler/rustc_interface/src/util.rs:159:32
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace. This is on 26873fedff6b77b9233a5437b67da4bfd0756e26 |
Finished benchmarking commit (2f4580eb83980655419fbe5c0ddaefdaf513d628): comparison url. Instruction count
Max RSS (memory usage)Results
CyclesResults
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. Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @bors rollup=never Footnotes |
@5225225: thanks for the info! So I would need to enable debug assertions locally to see the SIGILL? You got a backtrace when you ran it locally. Is there a reason why the crash on CI has so little information? |
I edited the debug assertions to be a panic instead of an abort, and then used See: rust/library/core/src/intrinsics.rs Lines 2120 to 2136 in 86c6ebe
You want to make that |
I've done some more debugging work, and it's a slice get_unchecked (using usize as an index) where the index is 0, and the slice length is 0. I highly suspect the changes to |
Right. I think I know why now. It's the get_unchecked. We're setting the length to 0, but that means you can't use .get_unchecked or it's UB. the set_len is correct, but you have to keep the pointer arithmetic. (To be polite, you could debug_assert that your pointer arithmetic offset is |
Oh, I understand what happened. Earlier in the year I wrote some code that moved Thanks again for the assistance, @5225225! |
26873fe
to
7b8036b
Compare
Some changes occurred in src/tools/clippy cc @rust-lang/clippy |
@bors try @rust-timer queue |
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
⌛ Trying commit 7b8036b96c28b0620e45df95aba0ac4e227fc718 with merge fe6140f28760415adc7f741133b30398a78a44ea... |
Finished benchmarking commit (fe7f01b53ec66f47a4b76ecf575e9d7cc0875d6f): comparison URL. Overall result: ❌✅ regressions and improvements - ACTION NEEDEDBenchmarking 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. Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @bors rollup=never Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis 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.
CyclesResultsThis 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.
Footnotes |
bd0be08
to
19e82aa
Compare
@bors try @rust-timer queue |
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
⌛ Trying commit 19e82aa4fe4005a5292e3c0f157fde2a5e8bada9 with merge 0375fd49348116907547d1840e4d5c6dc484a828... |
☀️ Try build successful - checks-actions |
Queued 0375fd49348116907547d1840e4d5c6dc484a828 with parent 2e35f95, future comparison URL. |
Finished benchmarking commit (0375fd49348116907547d1840e4d5c6dc484a828): comparison URL. Overall result: ❌✅ regressions and improvements - ACTION NEEDEDBenchmarking 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. Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @bors rollup=never Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis 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.
CyclesResultsThis 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.
Footnotes |
19e82aa
to
7b7fee4
Compare
@bors try @rust-timer queue |
Awaiting bors try build completion. @rustbot label: +S-waiting-on-perf |
⌛ Trying commit 7b7fee4 with merge bbb70faad6a3f11ab6a2bfa5ffe30c95b40161a9... |
☀️ Try build successful - checks-actions |
Queued bbb70faad6a3f11ab6a2bfa5ffe30c95b40161a9 with parent 8c41305, future comparison URL. |
Finished benchmarking commit (bbb70faad6a3f11ab6a2bfa5ffe30c95b40161a9): comparison URL. Overall result: ❌✅ regressions and improvements - ACTION NEEDEDBenchmarking 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. Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @bors rollup=never Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)ResultsThis 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.
CyclesResultsThis 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.
Footnotes |
@nnethercote you have this under control, correct? (just checking PRs without review activity for a while) |
r? @ghost