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

Make associated item collection a query #68837

Merged
merged 1 commit into from
Feb 6, 2020
Merged

Make associated item collection a query #68837

merged 1 commit into from
Feb 6, 2020

Conversation

jonas-schievink
Copy link
Contributor

Before this change, every time associated items were iterated over (which rustc does a lot – this can probably be further optimized), there would be N+1 queries to fetch all assoc. items. Now there's just one after they've been computed once.

@rust-highfive
Copy link
Collaborator

r? @estebank

(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 5, 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
Contributor

bors commented Feb 5, 2020

⌛ Trying commit 4fc4b95 with merge 694e0b9...

bors added a commit that referenced this pull request Feb 5, 2020
Make associated item collection a query

Before this change, every time associated items were iterated over (which rustc does *a lot* – this can probably be further optimized), there would be N+1 queries to fetch all assoc. items. Now there's just one after they've been computed once.
@estebank
Copy link
Contributor

estebank commented Feb 5, 2020

r=me after timer's done

@bors
Copy link
Contributor

bors commented Feb 5, 2020

☀️ Try build successful - checks-azure
Build commit: 694e0b9 (694e0b9eec970e6d7d2f12faa932312bb4676e42)

@rust-timer
Copy link
Collaborator

Queued 694e0b9 with parent c9290dc, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit 694e0b9, comparison URL.

@jonas-schievink
Copy link
Contributor Author

Some pretty nice improvements in packed-simd, and no real regressions. Nice.

@bors r=estebank

@bors
Copy link
Contributor

bors commented Feb 5, 2020

📌 Commit 4fc4b95 has been approved by estebank

@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 5, 2020
@Mark-Simulacrum
Copy link
Member

cc @eddyb I recall an issue you filed a while back about something like this

@@ -310,6 +310,11 @@ rustc_queries! {
/// Maps from a trait item to the trait item "descriptor".
query associated_item(_: DefId) -> ty::AssocItem {}

/// Collects the associated items defined on a trait or impl.
query associated_items(key: DefId) -> ty::AssocItemsIterator<'tcx> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be just the slice? You might need to replace this:

for x in tcx.associated_items(def_id)

with:

for &x in tcx.associated_items(def_id)

But for the most part it should be fine not to even do that.

@eddyb
Copy link
Member

eddyb commented Feb 6, 2020

@Mark-Simulacrum I don't recall, maybe it wasn't me. cc @nikomatsakis

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 6, 2020
…, r=estebank

Make associated item collection a query

Before this change, every time associated items were iterated over (which rustc does *a lot* – this can probably be further optimized), there would be N+1 queries to fetch all assoc. items. Now there's just one after they've been computed once.
bors added a commit that referenced this pull request Feb 6, 2020
Rollup of 9 pull requests

Successful merges:

 - #68691 (Remove `RefCell` usage from `ObligationForest`.)
 - #68751 (Implement `unused_parens` for `const` and `static` items)
 - #68788 (Towards unified `fn` grammar)
 - #68837 (Make associated item collection a query)
 - #68842 (or_patterns: add regression test for #68785)
 - #68844 (use def_path_str for missing_debug_impls message)
 - #68845 (stop using BytePos for computing spans in librustc_parse/parser/mod.rs)
 - #68869 (clean up E0271 explanation)
 - #68880 (Forbid using `0` as issue number)

Failed merges:

r? @ghost
@bors bors merged commit 4fc4b95 into rust-lang:master Feb 6, 2020
@jonas-schievink jonas-schievink deleted the assoc-item-lookup-2 branch February 6, 2020 21:43
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).
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/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-zz pushed a commit to Dylan-DPC-zz/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-zz pushed a commit to Dylan-DPC-zz/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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants