Skip to content

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

Merged
bors merged 3 commits into
rust-lang:masterfrom
michaelwoerister:cache-ty-collect
Jan 26, 2018
Merged

incr.comp.: Cache type_of and some other queries.#47455
bors merged 3 commits into
rust-lang:masterfrom
michaelwoerister:cache-ty-collect

Conversation

@michaelwoerister

@michaelwoeristermichaelwoerister commented Jan 15, 2018

Copy link
Copy Markdown
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

nikomatsakis commented Jan 16, 2018

Copy link
Copy Markdown
Contributor

@bors r+

@bors

bors commented Jan 16, 2018

Copy link
Copy Markdown
Collaborator

📌 Commit b726204 has been approved by nikomatsakis

@nikomatsakis

nikomatsakis commented Jan 16, 2018

Copy link
Copy Markdown
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

michaelwoerister commented Jan 16, 2018

Copy link
Copy Markdown
MemberAuthor

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.

@kennytmkennytm 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 ofrust-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
@kennytmkennytm mentioned this pull request Jan 17, 2018
Closed
@michaelwoerister

michaelwoerister commented Jan 17, 2018

Copy link
Copy Markdown
MemberAuthor

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

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

@kennytmkennytm 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

nikomatsakis commented Jan 17, 2018

Copy link
Copy Markdown
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

michaelwoerister commented Jan 18, 2018

Copy link
Copy Markdown
MemberAuthor

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

@nikomatsakis

nikomatsakis commented Jan 19, 2018

Copy link
Copy Markdown
Contributor

@bors r+

@bors

bors commented Jan 19, 2018

Copy link
Copy Markdown
Collaborator

📌 Commit 0f13dbd has been approved by nikomatsakis

@nikomatsakisnikomatsakis 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
@michaelwoeristermichaelwoerister mentioned this pull request Jan 22, 2018
Open
32 tasks
@bors

bors commented Jan 23, 2018

Copy link
Copy Markdown
Collaborator

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

@nikomatsakis

nikomatsakis commented Jan 23, 2018

Copy link
Copy Markdown
Contributor

@bors r+

(@michaelwoerister -- did you forget this?)

@bors

bors commented Jan 23, 2018

Copy link
Copy Markdown
Collaborator

📌 Commit 0a4f347 has been approved by nikomatsakis

@alexcrichtonalexcrichton 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:masterJan 26, 2018
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@nikomatsakisnikomatsakis

Labels

S-waiting-on-borsStatus: 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