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

cache test item names #8182

Merged
merged 1 commit into from
Dec 28, 2021
Merged

cache test item names #8182

merged 1 commit into from
Dec 28, 2021

Conversation

llogiq
Copy link
Contributor

@llogiq llogiq commented Dec 27, 2021

This avoids quadratic behavior (collecting all test item names for each eq_op instance within the module). However, it invests a good deal of memory to buy this speedup. If that becomes a problem, I may need to change the cache to only store the chain of last visited modules.

This hopefully fixes #8171.


changelog: none

@rust-highfive
Copy link

r? @giraffate

(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 Dec 27, 2021
Copy link
Contributor

@giraffate giraffate left a comment

Choose a reason for hiding this comment

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

LGTM with nits

clippy_utils/src/lib.rs Outdated Show resolved Hide resolved
clippy_utils/src/lib.rs Outdated Show resolved Hide resolved
@llogiq
Copy link
Contributor Author

llogiq commented Dec 28, 2021

@giraffate thanks, changed the name.

@giraffate
Copy link
Contributor

@bors r+

Thanks!

@bors
Copy link
Collaborator

bors commented Dec 28, 2021

📌 Commit 8da9a82 has been approved by giraffate

@bors
Copy link
Collaborator

bors commented Dec 28, 2021

⌛ Testing commit 8da9a82 with merge 3354876...

@bors
Copy link
Collaborator

bors commented Dec 28, 2021

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: giraffate
Pushing 3354876 to master...

@bors bors merged commit 3354876 into master Dec 28, 2021
@llogiq llogiq deleted the cache-test-items branch December 28, 2021 06:04
@llogiq
Copy link
Contributor Author

llogiq commented Dec 28, 2021

@wtfsck can you build master and check if this improves your clippy perf?

@wtfsck
Copy link

wtfsck commented Dec 28, 2021

@llogiq Yes, this fixed the perf problems and the speed is back to normal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cargo clippy is sometimes >20x slower than cargo check (since 1.58.0)
5 participants