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.

Sign up for GitHub

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

Jump to bottom

incr.comp.: Cache type_of and some other queries. #47455

Merged
merged 3 commits into from
Jan 26, 2018
Merged

incr.comp.: Cache type_of and some other queries. #47455

merged 3 commits into from
Jan 26, 2018

Conversation

michaelwoerister
Copy link
Member

Cache some more queries that show up high during profiling. Does not include generics_of yet (which profiling shows to also take up quite some time) because the type_param_to_index field in there uses raw DefIndex values from other crates.

r? @nikomatsakis

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jan 16, 2018

📌 Commit b726204 has been approved by nikomatsakis

@nikomatsakis
Copy link
Contributor

because the type_param_to_index field in there uses raw DefIndex values from other crates.

say a bit more about this? is this a bug to be closed?

@michaelwoerister
Copy link
Member Author

The incr.comp. cache serializer does not accept raw DefIndex values because it cannot make sense of them without the corresponding CrateNum. I plan to fix this by creating a special data structure with custom serialization for this lookup table (or I'll open an issue with mentoring instruction).

It's not a really bug though because we have an assertion against accidentally running into trouble there.

@kennytm kennytm added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jan 17, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Jan 17, 2018
@kennytm
Rollup merge of rust-lang#47455 - michaelwoerister:cache-ty-collect, …
b3ff458 
…r=nikomatsakis

incr.comp.: Cache type_of and some other queries.

Cache some more queries that show up high during profiling. Does not include `generics_of` yet (which profiling shows to also take up quite some time) because the `type_param_to_index` field in there uses raw `DefIndex` values from other crates.

r? @nikomatsakis
@kennytm kennytm mentioned this pull request Jan 17, 2018
Closed
@michaelwoerister
Copy link
Member Author

@nikomatsakis, I pushed two more commits to also cache the generics_of query.

@kennytm, I hope this doesn't mess with your rollup.

@kennytm kennytm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 17, 2018
@nikomatsakis
Copy link
Contributor

It's not a really bug though because we have an assertion against accidentally running into trouble there.

well, the "bug" is probably just not using DefIndex anymore (which imo is not a type we should directly use; I'd prefer e.g. LocalDefId), no?

@michaelwoerister
Copy link
Member Author

I switched it to regular DefIds in the additional commits (since we can encounter ty::Generics from other crates).

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jan 19, 2018

📌 Commit 0f13dbd has been approved by nikomatsakis

@nikomatsakis nikomatsakis 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 Jan 19, 2018
@michaelwoerister michaelwoerister mentioned this pull request Jan 22, 2018
Open
32 tasks
@bors
Copy link
Contributor

bors commented Jan 23, 2018

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

@nikomatsakis
Copy link
Contributor

@bors r+

(@michaelwoerister -- did you forget this?)

@bors
Copy link
Contributor

bors commented Jan 23, 2018

📌 Commit 0a4f347 has been approved by nikomatsakis

@alexcrichton alexcrichton mentioned this pull request Jan 25, 2018
Merged
bors added a commit that referenced this pull request Jan 26, 2018
@bors
Auto merge of #47748 - alexcrichton:rollup, r=alexcrichton
bacb5c5 
Rollup of 19 pull requests

- Successful merges: #47415, #47437, #47439, #47453, #47460, #47502, #47529, #47600, #47607, #47618, #47626, #47656, #47668, #47696, #47701, #47705, #47710, #47711, #47719
- Failed merges: #47455, #47521
@bors bors merged commit 0a4f347 into rust-lang:master Jan 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@nikomatsakis nikomatsakis

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@michaelwoerister @nikomatsakis @bors @kennytm