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

[HACK] metadata/rmeta: skip sorting impls by DefPathHashes. #70462

Closed
wants to merge 1 commit into from

Conversation

eddyb
Copy link
Member

@eddyb eddyb commented Mar 27, 2020

See #69060 (comment) for some background.

I'm mostly opening this to do some experimentation (which will require multiple perf runs using the same base, so I'll probably start try builds in parallel using a second PR).

I'd prefer the solution described in #69060 (comment), as it should result in better accuracy as well, but that's more work than this quick hack, at least for testing purposes.

cc @nikomatsakis @michaelwoerister @Mark-Simulacrum

@rust-highfive
Copy link
Collaborator

r? @petrochenkov

(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 27, 2020
@eddyb eddyb mentioned this pull request Mar 27, 2020
@eddyb
Copy link
Member Author

eddyb commented Mar 27, 2020

@bors try @rust-timer queue

@rust-timer
Copy link
Collaborator

Awaiting bors try build completion

@bors

This comment has been minimized.

@eddyb

This comment has been minimized.

@bors

This comment has been minimized.

@bors bors 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 Mar 27, 2020
@eddyb
Copy link
Member Author

eddyb commented Mar 27, 2020

@bors try (oops, started the other build too soon)

@bors
Copy link
Contributor

bors commented Mar 27, 2020

⌛ Trying commit d6b49f9 with merge 335898da9076089c34ec0596e12cc7de600ff03b...

@eddyb
Copy link
Member Author

eddyb commented Mar 27, 2020

(try builds both seem to be running, common merge parent 697d6b3)

@bors
Copy link
Contributor

bors commented Mar 27, 2020

☀️ Try build successful - checks-azure
Build commit: 335898da9076089c34ec0596e12cc7de600ff03b (335898da9076089c34ec0596e12cc7de600ff03b)

@rust-timer
Copy link
Collaborator

Queued 335898da9076089c34ec0596e12cc7de600ff03b with parent 697d6b3, future comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit 335898da9076089c34ec0596e12cc7de600ff03b, comparison URL.

@eddyb
Copy link
Member Author

eddyb commented Mar 27, 2020

Comparison between "this PR" and "this PR + empty commit".

Nothing I've looked at has any query count differences, which is nice, but there's still a lot of noise from codegen/LLVM, so that needs to be looked into separately. I suppose I was wrong to assume that the query counts and noisy codegen/LLVM were related.

@petrochenkov
Copy link
Contributor

r? @ghost

@petrochenkov petrochenkov removed their assignment Mar 28, 2020
@eddyb eddyb closed this Apr 8, 2020
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants