Improve performance of coherence checks #68966

jonas-schievink · 2020-02-08T20:55:29Z

The biggest perf improvement in here is expected to come from the removal of the remaining #43355 warning code since that effectively runs the expensive parts of coherence twice, which can even be visualized by obtaining a flamegraph:

rust-highfive · 2020-02-08T20:55:33Z

r? @varkor

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

jonas-schievink · 2020-02-08T20:55:50Z

@bors try @rust-timer queue

rust-timer · 2020-02-08T20:55:52Z

Awaiting bors try build completion

bors · 2020-02-08T20:56:01Z

⌛ Trying commit 14a15d6 with merge fe57f83...

Improve performance of coherence checks The biggest perf improvement in here is expected to come from the removal of the remaining #43355 warning code since the effectively runs the expensive parts of coherence *twice* (which can even be visualized by obtaining a flamegraph https://twitter.com/sheevink/status/1225963187511709696).

Mark-Simulacrum · 2020-02-08T20:59:17Z

(Edited description to inline the image link to hopefully have it not get lost to time as easily)

bors · 2020-02-08T23:52:32Z

☀️ Try build successful - checks-azure
Build commit: fe57f83 (fe57f83be5309ba42c3dccaa488d1dad3ca6d863)

rust-timer · 2020-02-08T23:52:34Z

Queued fe57f83 with parent 85ffd44, future comparison URL.

varkor · 2020-02-09T00:36:55Z

r=me if perf looks good.

jonas-schievink · 2020-02-09T11:55:13Z

5-8% improvement in packed-simd, no regressions.

@bors r=varkor

bors · 2020-02-09T11:55:14Z

📌 Commit 14a15d6 has been approved by varkor

This has negligible perf impact, but it does improve the code a bit. * Only query the specialization graph of any trait once instead of once per impl * Loop over impls only once, precomputing impl DefId and TraitRef

This was showing up in profiles. Also deduplicates the code to get just the impl header span.

This was previously a future-compat warning that has since been turned into hard error, but without actually removing all the code. Avoids a call to `traits::overlapping_impls`, which is expensive.

jonas-schievink · 2020-02-09T22:24:31Z

@bors r=varkor

bors · 2020-02-09T22:24:32Z

📌 Commit 2309592 has been approved by varkor

…arkor Improve performance of coherence checks The biggest perf improvement in here is expected to come from the removal of the remaining rust-lang#43355 warning code since that effectively runs the expensive parts of coherence *twice*, which can even be visualized by obtaining a flamegraph: ![image](https://user-images.githubusercontent.com/5047365/74091959-e1f41200-4a8b-11ea-969d-2849d3f86c63.png)

@ghost

Rollup of 6 pull requests Successful merges: - #68694 (Reduce the number of `RefCell`s in `InferCtxt`.) - #68966 (Improve performance of coherence checks) - #68976 (Make `num::NonZeroX::new` an unstable `const fn`) - #68992 (Correctly parse `mut a @ b`) - #69005 (Small graphviz improvements for the new dataflow framework) - #69006 (parser: Keep current and previous tokens precisely) Failed merges: r? @ghost

Pkgsrc changes: * Bump rust bootstrap version to 1.42.0, except for Darwin/i686 where the bootstrap is not (yet?) available. Upstream changes: Version 1.43.0 (2020-04-23) ========================== Language -------- - [Fixed using binary operations with `&{number}` (e.g. `&1.0`) not having the type inferred correctly.][68129] - [Attributes such as `#[cfg()]` can now be used on `if` expressions.][69201] **Syntax only changes** - [Allow `type Foo: Ord` syntactically.][69361] - [Fuse associated and extern items up to defaultness.][69194] - [Syntactically allow `self` in all `fn` contexts.][68764] - [Merge `fn` syntax + cleanup item parsing.][68728] - [`item` macro fragments can be interpolated into `trait`s, `impl`s, and `extern` blocks.][69366] For example, you may now write: ```rust macro_rules! mac_trait { ($i:item) => { trait T { $i } } } mac_trait! { fn foo() {} } ``` These are still rejected *semantically*, so you will likely receive an error but these changes can be seen and parsed by macros and conditional compilation. Compiler -------- - [You can now pass multiple lint flags to rustc to override the previous flags.][67885] For example; `rustc -D unused -A unused-variables` denies everything in the `unused` lint group except `unused-variables` which is explicitly allowed. However, passing `rustc -A unused-variables -D unused` denies everything in the `unused` lint group **including** `unused-variables` since the allow flag is specified before the deny flag (and therefore overridden). - [rustc will now prefer your system MinGW libraries over its bundled libraries if they are available on `windows-gnu`.][67429] - [rustc now buffers errors/warnings printed in JSON.][69227] Libraries --------- - [`Arc<[T; N]>`, `Box<[T; N]>`, and `Rc<[T; N]>`, now implement `TryFrom<Arc<[T]>>`,`TryFrom<Box<[T]>>`, and `TryFrom<Rc<[T]>>` respectively.][69538] **Note** These conversions are only available when `N` is `0..=32`. - [You can now use associated constants on floats and integers directly, rather than having to import the module.][68952] e.g. You can now write `u32::MAX` or `f32::NAN` with no imports. - [`u8::is_ascii` is now `const`.][68984] - [`String` now implements `AsMut<str>`.][68742] - [Added the `primitive` module to `std` and `core`.][67637] This module reexports Rust's primitive types. This is mainly useful in macros where you want avoid these types being shadowed. - [Relaxed some of the trait bounds on `HashMap` and `HashSet`.][67642] - [`string::FromUtf8Error` now implements `Clone + Eq`.][68738] Stabilized APIs --------------- - [`Once::is_completed`] - [`f32::LOG10_2`] - [`f32::LOG2_10`] - [`f64::LOG10_2`] - [`f64::LOG2_10`] - [`iter::once_with`] Cargo ----- - [You can now set config `[profile]`s in your `.cargo/config`, or through your environment.][cargo/7823] - [Cargo will now set `CARGO_BIN_EXE_<name>` pointing to a binary's executable path when running integration tests or benchmarks.][cargo/7697] `<name>` is the name of your binary as-is e.g. If you wanted the executable path for a binary named `my-program`you would use `env!("CARGO_BIN_EXE_my-program")`. Misc ---- - [Certain checks in the `const_err` lint were deemed unrelated to const evaluation][69185], and have been moved to the `unconditional_panic` and `arithmetic_overflow` lints. Compatibility Notes ------------------- - [Having trailing syntax in the `assert!` macro is now a hard error.][69548] This has been a warning since 1.36.0. - [Fixed `Self` not having the correctly inferred type.][69340] This incorrectly led to some instances being accepted, and now correctly emits a hard error. [69340]: rust-lang/rust#69340 Internal Only ------------- These changes provide no direct user facing benefits, but represent significant improvements to the internals and overall performance of `rustc` and related tools. - [All components are now built with `opt-level=3` instead of `2`.][67878] - [Improved how rustc generates drop code.][67332] - [Improved performance from `#[inline]`-ing certain hot functions.][69256] - [traits: preallocate 2 Vecs of known initial size][69022] - [Avoid exponential behaviour when relating types][68772] - [Skip `Drop` terminators for enum variants without drop glue][68943] - [Improve performance of coherence checks][68966] - [Deduplicate types in the generator witness][68672] - [Invert control in struct_lint_level.][68725] [67332]: rust-lang/rust#67332 [67429]: rust-lang/rust#67429 [67637]: rust-lang/rust#67637 [67642]: rust-lang/rust#67642 [67878]: rust-lang/rust#67878 [67885]: rust-lang/rust#67885 [68129]: rust-lang/rust#68129 [68672]: rust-lang/rust#68672 [68725]: rust-lang/rust#68725 [68728]: rust-lang/rust#68728 [68738]: rust-lang/rust#68738 [68742]: rust-lang/rust#68742 [68764]: rust-lang/rust#68764 [68772]: rust-lang/rust#68772 [68943]: rust-lang/rust#68943 [68952]: rust-lang/rust#68952 [68966]: rust-lang/rust#68966 [68984]: rust-lang/rust#68984 [69022]: rust-lang/rust#69022 [69185]: rust-lang/rust#69185 [69194]: rust-lang/rust#69194 [69201]: rust-lang/rust#69201 [69227]: rust-lang/rust#69227 [69548]: rust-lang/rust#69548 [69256]: rust-lang/rust#69256 [69361]: rust-lang/rust#69361 [69366]: rust-lang/rust#69366 [69538]: rust-lang/rust#69538 [cargo/7823]: rust-lang/cargo#7823 [cargo/7697]: rust-lang/cargo#7697 [`Once::is_completed`]: https://doc.rust-lang.org/std/sync/struct.Once.html#method.is_completed [`f32::LOG10_2`]: https://doc.rust-lang.org/std/f32/consts/constant.LOG10_2.html [`f32::LOG2_10`]: https://doc.rust-lang.org/std/f32/consts/constant.LOG2_10.html [`f64::LOG10_2`]: https://doc.rust-lang.org/std/f64/consts/constant.LOG10_2.html [`f64::LOG2_10`]: https://doc.rust-lang.org/std/f64/consts/constant.LOG2_10.html [`iter::once_with`]: https://doc.rust-lang.org/std/iter/fn.once_with.html

rust-highfive assigned varkor Feb 8, 2020

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

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 9, 2020

Dylan-DPC-zz mentioned this pull request Feb 9, 2020

Rollup of 4 pull requests #68996

Closed

jonas-schievink added 4 commits February 9, 2020 23:17

Reduce queries/map lookups done by coherence

5ab1ab4

This has negligible perf impact, but it does improve the code a bit. * Only query the specialization graph of any trait once instead of once per impl * Loop over impls only once, precomputing impl DefId and TraitRef

Add desc to specialization_graph_of query

82d6e79

Defer error span calculation until needed

8f468ee

This was showing up in profiles. Also deduplicates the code to get just the impl header span.

Remove vestigial #43355-compat code

2309592

This was previously a future-compat warning that has since been turned into hard error, but without actually removing all the code. Avoids a call to `traits::overlapping_impls`, which is expensive.

Dylan-DPC-zz mentioned this pull request Feb 10, 2020

Rollup of 6 pull requests #69012

Merged

bors merged commit 2309592 into rust-lang:master Feb 10, 2020

jonas-schievink deleted the coherence-perf branch March 13, 2020 16:06

jonas-schievink added relnotes Marks issues that should be documented in the release notes of the next release. relnotes-perf Performance improvements that should be mentioned in the release notes labels Mar 13, 2020

jonas-schievink modified the milestones: 1.44, 1.43 Mar 13, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve performance of coherence checks #68966

Improve performance of coherence checks #68966

jonas-schievink commented Feb 8, 2020 •

edited

rust-highfive commented Feb 8, 2020

jonas-schievink commented Feb 8, 2020

rust-timer commented Feb 8, 2020

bors commented Feb 8, 2020

Mark-Simulacrum commented Feb 8, 2020

bors commented Feb 8, 2020

rust-timer commented Feb 8, 2020

varkor commented Feb 9, 2020

jonas-schievink commented Feb 9, 2020

bors commented Feb 9, 2020

jonas-schievink commented Feb 9, 2020

bors commented Feb 9, 2020

Improve performance of coherence checks #68966

Improve performance of coherence checks #68966

Conversation

jonas-schievink commented Feb 8, 2020 • edited

rust-highfive commented Feb 8, 2020

jonas-schievink commented Feb 8, 2020

rust-timer commented Feb 8, 2020

bors commented Feb 8, 2020

Mark-Simulacrum commented Feb 8, 2020

bors commented Feb 8, 2020

rust-timer commented Feb 8, 2020

varkor commented Feb 9, 2020

jonas-schievink commented Feb 9, 2020

bors commented Feb 9, 2020

jonas-schievink commented Feb 9, 2020

bors commented Feb 9, 2020

jonas-schievink commented Feb 8, 2020 •

edited