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

Perform HIR indexing per-owner #82681

Closed
wants to merge 2 commits into from
Closed

Conversation

cjgillot
Copy link
Contributor

@cjgillot cjgillot commented Mar 1, 2021

The aim is to have HIR indexing more fine-grained, and to perform it for each HIR owner independently.

The subtle part is to find an owner's parent HirId.
The first few commits make it so that an owner's parent's owner is given by def_key.parent.
Then, we just need to record the nested owners inside index_hir to find the right local id.

@rust-highfive
Copy link
Collaborator

r? @lcnr

(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 Mar 1, 2021
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@cjgillot
Copy link
Contributor Author

cjgillot commented Mar 3, 2021

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 3, 2021
@bors
Copy link
Contributor

bors commented Mar 3, 2021

⌛ Trying commit fa3d6e006d3018be896bd1d3079831716535894c with merge f1126bb79abbc4fd7bd3c930d171bf9f0404a2ee...

@bors
Copy link
Contributor

bors commented Mar 3, 2021

☀️ Try build successful - checks-actions
Build commit: f1126bb79abbc4fd7bd3c930d171bf9f0404a2ee (f1126bb79abbc4fd7bd3c930d171bf9f0404a2ee)

@rust-timer
Copy link
Collaborator

Queued f1126bb79abbc4fd7bd3c930d171bf9f0404a2ee with parent 476acbf, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (f1126bb79abbc4fd7bd3c930d171bf9f0404a2ee): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 4, 2021
@cjgillot
Copy link
Contributor Author

cjgillot commented Mar 4, 2021

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 4, 2021
@bors
Copy link
Contributor

bors commented Mar 4, 2021

⌛ Trying commit 85adc7842d1ec908a02627cb1648aede38aa1bd6 with merge 97823b8d6cd471a09fad9755d8f9ed30cd1f51f4...

@bors
Copy link
Contributor

bors commented Mar 4, 2021

☀️ Try build successful - checks-actions
Build commit: 97823b8d6cd471a09fad9755d8f9ed30cd1f51f4 (97823b8d6cd471a09fad9755d8f9ed30cd1f51f4)

@rust-timer
Copy link
Collaborator

Queued 97823b8d6cd471a09fad9755d8f9ed30cd1f51f4 with parent ec7f258, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (97823b8d6cd471a09fad9755d8f9ed30cd1f51f4): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf

@rustbot rustbot removed the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Mar 4, 2021
@cjgillot
Copy link
Contributor Author

cjgillot commented Mar 5, 2021

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@cjgillot cjgillot mentioned this pull request Aug 20, 2021
11 tasks
) -> Option<IndexedHir<'hir>> {
let item = *krate.owners.get(owner)?.as_ref()?;
let hash = hash_body(&mut hcx, item);
let mut nodes = IndexVec::new();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
let mut nodes = IndexVec::new();
let mut nodes = IndexVec::with_capacity(1);

Similar thing here

let mut nodes = IndexVec::new();
nodes.push(Some(ParentedNode { parent: ItemLocalId::new(0), node: node.into() }));

But i don't think this will give observable results, really.

@inquisitivecrystal inquisitivecrystal added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Aug 24, 2021
@cjgillot cjgillot changed the title [WIP] Perform HIR indexing per-owner Perform HIR indexing per-owner Aug 25, 2021
@Dylan-DPC-zz Dylan-DPC-zz removed the S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. label Aug 27, 2021
@Dylan-DPC-zz
Copy link

this is no longer blocked, as the PR is merged

@Dylan-DPC-zz Dylan-DPC-zz added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 27, 2021
@bors
Copy link
Contributor

bors commented Aug 28, 2021

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

@cjgillot
Copy link
Contributor Author

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

@bors
Copy link
Contributor

bors commented Aug 28, 2021

⌛ Trying commit 7e7e7366c524aca8fd4a4a2d9455de438fc42338 with merge 5e429138294dd111c5314026c1f7def4a17b02e4...

@bors
Copy link
Contributor

bors commented Aug 28, 2021

☀️ Try build successful - checks-actions
Build commit: 5e429138294dd111c5314026c1f7def4a17b02e4 (5e429138294dd111c5314026c1f7def4a17b02e4)

@rust-timer
Copy link
Collaborator

Queued 5e429138294dd111c5314026c1f7def4a17b02e4 with parent 05cccdc, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit (5e429138294dd111c5314026c1f7def4a17b02e4): comparison url.

Summary: ERROR categorizing benchmark run!

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed perf-regression Performance regression. labels Aug 28, 2021
@Aaron1011 Aaron1011 removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 5, 2021
@bors
Copy link
Contributor

bors commented Sep 6, 2021

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

@bors
Copy link
Contributor

bors commented Sep 19, 2021

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

@cjgillot
Copy link
Contributor Author

Closing in favour of #89124

@cjgillot cjgillot closed this Sep 20, 2021
@cjgillot cjgillot deleted the incr-index branch September 20, 2021 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.