Skip to content

Conversation

jonas-schievink
Copy link
Contributor

@jonas-schievink jonas-schievink commented Feb 8, 2020

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:

image

@rust-highfive
Copy link
Contributor

r? @varkor

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 8, 2020
@jonas-schievink
Copy link
Contributor Author

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors
Copy link
Collaborator

bors commented Feb 8, 2020

⌛ Trying commit 14a15d6 with merge fe57f83...

bors added a commit that referenced this pull request Feb 8, 2020
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
Copy link
Member

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

@bors
Copy link
Collaborator

bors commented Feb 8, 2020

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

@rust-timer
Copy link
Collaborator

Queued fe57f83 with parent 85ffd44, future comparison URL.

@varkor
Copy link
Contributor

varkor commented Feb 9, 2020

r=me if perf looks good.

@jonas-schievink
Copy link
Contributor Author

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

@bors r=varkor

@bors
Copy link
Collaborator

bors commented Feb 9, 2020

📌 Commit 14a15d6 has been approved by varkor

@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 9, 2020
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
Copy link
Contributor Author

@bors r=varkor

@bors
Copy link
Collaborator

bors commented Feb 9, 2020

📌 Commit 2309592 has been approved by varkor

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 10, 2020
…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)
bors added a commit that referenced this pull request Feb 10, 2020
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
@bors bors merged commit 2309592 into rust-lang:master Feb 10, 2020
@jonas-schievink jonas-schievink deleted the coherence-perf branch March 13, 2020 16:06
@jonas-schievink 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 jonas-schievink modified the milestones: 1.44, 1.43 Mar 13, 2020
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request May 16, 2020
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

6 participants