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

HIR: introduce a HirId to DefId map in Definitions #63849

Open
wants to merge 1 commit into
base: master
from

Conversation

@ljedrz
Copy link
Contributor

commented Aug 24, 2019

Split out from #62975.

Introduce a HirId to DefIndex map in map::Definitions; this introduces a little bit of extra complexity, but could result in a performance win. I'd do a perf run to check if it's worth it.

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

commented Aug 24, 2019

r? @cramertj

(rust_highfive has picked a reviewer for you, use r? to override)

@ljedrz

This comment has been minimized.

Copy link
Contributor Author

commented Aug 24, 2019

@bors try

@bors

This comment has been minimized.

Copy link
Contributor

commented Aug 24, 2019

⌛️ Trying commit f71703a with merge 8a50d3d...

bors added a commit that referenced this pull request Aug 24, 2019
Auto merge of #63849 - ljedrz:kill_off_hir_to_node_id2, r=<try>
HIR: introduce a HirId to DefId map in Definitions

Split out from #62975.

Introduce a `HirId` to `DefIndex` map in `map::Definitions`; this introduces a little bit of extra complexity, but could result in a performance win. I'd do a perf run to check if it's worth it.
@bors

This comment has been minimized.

Copy link
Contributor

commented Aug 24, 2019

☀️ Try build successful - checks-azure
Build commit: 8a50d3d

@cramertj

This comment has been minimized.

Copy link
Member

commented Aug 26, 2019

r? @Zoxc

@rust-highfive rust-highfive assigned Zoxc and unassigned cramertj Aug 26, 2019

@bjorn3

This comment has been minimized.

Copy link
Contributor

commented Sep 1, 2019

This should get a perf run, shouldn't it?

@ljedrz

This comment has been minimized.

Copy link
Contributor Author

commented Sep 1, 2019

Yeah, I'd suggest doing that; I don't have perf creds, though.

@Mark-Simulacrum

This comment has been minimized.

Copy link
Member

commented Sep 1, 2019

@rust-timer build 8a50d3d

Feel free to ping me if you need a perf run

@rust-timer

This comment has been minimized.

Copy link

commented Sep 1, 2019

Success: Queued 8a50d3d with parent 4784645, comparison URL.

@rust-timer

This comment has been minimized.

Copy link

commented Sep 2, 2019

Finished benchmarking try commit 8a50d3d, comparison URL.

@ljedrz

This comment has been minimized.

Copy link
Contributor Author

commented Sep 2, 2019

Not too shabby; @Zoxc you said you'd recommend someone else to review this specific change - can you suggest someone?

@joelpalmer

This comment has been minimized.

Copy link

commented Sep 9, 2019

Ping from Triage: Hi @Zoxc - can we get an update on the review or the hunt for a reviewer? Also will cc @varkor since they are being suggested as a reviewer. Possibly they can help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
9 participants
You can’t perform that action at this time.