Skip to content
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

Speed up the inherent impl overlap check #68911

Merged
merged 5 commits into from Feb 9, 2020

Conversation

@jonas-schievink
Copy link
Member

jonas-schievink commented Feb 7, 2020

This gives a ~7% improvement in compile times for the stm32f0(x2) crate.

Also addresses @eddyb's comment in #68837 (comment).

@jonas-schievink

This comment has been minimized.

Copy link
Member Author

jonas-schievink commented Feb 7, 2020

@bors try @rust-timer queue

@rust-timer

This comment has been minimized.

Copy link

rust-timer commented Feb 7, 2020

Awaiting bors try build completion

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Feb 7, 2020

⌛️ Trying commit 000243c with merge a5b4dc5...

bors added a commit that referenced this pull request Feb 7, 2020
Speed up the inherent impl overlap check

This gives a ~7% improvement in compile times for the stm32f0(x2) crate.

Also addresses @eddyb's comment in #68837 (comment).
@bors

This comment has been minimized.

Copy link
Contributor

bors commented Feb 7, 2020

☀️ Try build successful - checks-azure
Build commit: a5b4dc5 (a5b4dc5b68506613cdad1d3cc74b89fc42133dbf)

@rust-timer

This comment has been minimized.

Copy link

rust-timer commented Feb 7, 2020

Queued a5b4dc5 with parent 442ae7f, future comparison URL.

@@ -666,7 +666,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

// The `Future` trait has only one associted item, `Output`,
// so check that this is what we see.
let output_assoc_item = self.tcx.associated_items(future_trait).nth(0).unwrap().def_id;
let output_assoc_item = self.tcx.associated_items(future_trait)[0].def_id;

This comment has been minimized.

Copy link
@eddyb

eddyb Feb 7, 2020

Member

I think this pattern (or worse, looking things up by name) should be replaced with lang items.

This comment has been minimized.

Copy link
@jonas-schievink

jonas-schievink Feb 7, 2020

Author Member

Yeah, definintely. This was a huge pain when working on generators, since the declaration order of the assoc. types is significant. Not something I'll try to fit in this PR though.

src/librustc/ty/mod.rs Outdated Show resolved Hide resolved
@jonas-schievink

This comment has been minimized.

Copy link
Member Author

jonas-schievink commented Feb 7, 2020

Looks like this made packed-simd build up to 15% faster in the best case, nice! (and no real regressions)

@jonas-schievink jonas-schievink force-pushed the jonas-schievink:inherent-overlap branch from 3a1125e to f95af65 Feb 7, 2020
@bors

This comment has been minimized.

Copy link
Contributor

bors commented Feb 8, 2020

☔️ The latest upstream changes (presumably #65232) made this pull request unmergeable. Please resolve the merge conflicts.

@petrochenkov

This comment has been minimized.

Copy link
Contributor

petrochenkov commented Feb 8, 2020

r=me after rebase

This reduces the number of `associated_item` queries done here.
Quickly skip impls that do not define any items with the same name
@jonas-schievink jonas-schievink force-pushed the jonas-schievink:inherent-overlap branch from f95af65 to 58a9284 Feb 8, 2020
@jonas-schievink

This comment has been minimized.

Copy link
Member Author

jonas-schievink commented Feb 8, 2020

@bors r=petrochenkov

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Feb 8, 2020

📌 Commit 58a9284 has been approved by petrochenkov

Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Feb 8, 2020
…=petrochenkov

Speed up the inherent impl overlap check

This gives a ~7% improvement in compile times for the stm32f0(x2) crate.

Also addresses @eddyb's comment in rust-lang#68837 (comment).
bors added a commit that referenced this pull request Feb 8, 2020
Rollup of 7 pull requests

Successful merges:

 - #68452 (Implement proper C ABI lowering for RISC-V)
 - #68834 (Fix and test implementation of BTreeMap's first/last_entry, pop_first/last)
 - #68881 (rustc_codegen_llvm: always set AlwaysPreserve on all debuginfo variables)
 - #68884 (Make the `type_of` return a generic type for generators)
 - #68911 (Speed up the inherent impl overlap check)
 - #68913 (Pretty-print generic params and where clauses on associated types)
 - #68918 (Don't use the word "unwrap" to describe "unwrap" methods)

Failed merges:

r? @ghost
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Feb 8, 2020
…=petrochenkov

Speed up the inherent impl overlap check

This gives a ~7% improvement in compile times for the stm32f0(x2) crate.

Also addresses @eddyb's comment in rust-lang#68837 (comment).
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Feb 9, 2020
…=petrochenkov

Speed up the inherent impl overlap check

This gives a ~7% improvement in compile times for the stm32f0(x2) crate.

Also addresses @eddyb's comment in rust-lang#68837 (comment).
jonas-schievink added a commit to jonas-schievink/rust that referenced this pull request Feb 9, 2020
…=petrochenkov

Speed up the inherent impl overlap check

This gives a ~7% improvement in compile times for the stm32f0(x2) crate.

Also addresses @eddyb's comment in rust-lang#68837 (comment).
bors added a commit that referenced this pull request Feb 9, 2020
Rollup of 5 pull requests

Successful merges:

 - #68738 (Derive Clone + Eq for std::string::FromUtf8Error)
 - #68742 (implement AsMut<str> for String)
 - #68881 (rustc_codegen_llvm: always set AlwaysPreserve on all debuginfo variables)
 - #68911 (Speed up the inherent impl overlap check)
 - #68913 (Pretty-print generic params and where clauses on associated types)

Failed merges:

r? @ghost
@bors bors merged commit 58a9284 into rust-lang:master Feb 9, 2020
4 checks passed
4 checks passed
pr Build #20200208.25 succeeded
Details
pr (Linux mingw-check) Linux mingw-check succeeded
Details
pr (Linux x86_64-gnu-llvm-7) Linux x86_64-gnu-llvm-7 succeeded
Details
pr (Linux x86_64-gnu-tools) Linux x86_64-gnu-tools succeeded
Details
krishna-veerareddy added a commit to krishna-veerareddy/rust-clippy that referenced this pull request Feb 9, 2020
bors added a commit to rust-lang/rust-clippy that referenced this pull request Feb 9, 2020
bors added a commit that referenced this pull request Feb 9, 2020
Make the inherent impl overlap check linear-time

Despite #68911, this code was still showing up in profiles, so I turned it from a O(n²) comparison between all inherent impls to an O(n) algorithm that builds an intermediate hash map to find items with the same names. It also makes the code a bit clearer.

The actual performance gains I've measured are pretty small (1-3%), and it's not impossible that the hash map has a negative perf impact in some situations, so I'll query perf for this.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 10, 2020
Changes:
````
Use current_dir instead of cargo_metadata
Fix run-pass tests when CARGO_TARGET_DIR is not set
Use `PATH` environment variable for testing
Rustup to rust-lang#68911
Allow `option-env-unwrap` within external macros
Account for `expect` being used to unwrap `Option`
Fix error E0460 when compiled on Rustc repo
Rustup "index ReEmpty by universe"
Add `option-env-unwrap` lint
Update CARGO_TARGET_DIR
Rustup rust-lang#67359
dev: Move DOCS_LINK out of lazy_static and reuse it
dev: Make UpdateMode a copy type
dev: Prefer `fs::read*` and improvement to replace text region
Rustup to rust-lang#68788
Rename ctx->cx in needless_continue
Improve help message in needless_continue
Fix rebase fallout
Document the indent_relative_to arg of snippet_block
Update remaining test files
Move tests to the end of the file in utils mod.rs
Update needless_continue stderr
Rewrite suggestion generation of needless_continue
Update block_in_if_condition test files
Make block_in_if_condition auto applicable
Optionally indent snippet_block relative to an Expr
dev: Use bytecount for faster line count
add excessive bools lints
Use lazy_static
move is_trait_impl_item check from functions.rs to utils
Add `serde_derive` to the need-to-be-disambiguated-crates list
Fix dogfood to use cargo mod too
compile-test: Handle CARGO_TARGET_DIR and transitive deps
Prevent failing to restart setup-toolchain
Few improvement to `utils::conf` module
Manage macros case + move to MaybeIncorrect when binding values
Merge fixes
Add wild and struct handling
Use span_lint_and_sugg + move infaillible lint
Re-cover use of unnecessary unwraps in macros
Add new lint: match with a single binding statement
Do not lint `unnecessary_unwrap` in macros
Remove unnecessary comment
improve 'iter_nth_zero' documentation
Use `checked_sub` to avoid index out of bounds
Decrease line length limit for stderrs
Split up `indexing_slicing` ui test
Move debug_assertions_with_mut_call to nursery
Add test for `await` in `debug_assert!(..)`
Don't trigger `debug_assert_with_mut_call` on `.await`
Small refactor of mutable_debug_assertions
Split up `drop_forget_ref` ui test
Pin versions to trigger lint correctly
````
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.