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

speed up cache by approximately 42x by avoiding pathlib #1953

Merged
merged 1 commit into from Feb 4, 2021
Merged

speed up cache by approximately 42x by avoiding pathlib #1953

merged 1 commit into from Feb 4, 2021

Conversation

asottile
Copy link
Contributor

@asottile asottile commented Feb 1, 2021

resolves #1950

Copy link
Collaborator

@cooperlees cooperlees left a comment

Choose a reason for hiding this comment

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

Thanks - Crazy increase. Expensive object this pathlib :D.

(will let some others look before merging)

@JelleZijlstra
Copy link
Collaborator

Looks like Travis is just never finishing, do we still need it?

@JelleZijlstra
Copy link
Collaborator

And thanks for catching and fixing this!

@ichard26
Copy link
Collaborator

ichard26 commented Feb 1, 2021

Looks like Travis is just never finishing, do we still need it?

Travis CI for Open Source is shutting down in a few weeks so the queue for jobs is insane due to lower resources. I'm 99.99% sure we don't need it as our Test, Lint, Docs, Upload / Package, Primer, and Fuzz workflows are all on GitHub Actions. So even though we can migrate to the .com version with its 1000 free Linux minutes(?), I don't think we need to.

(more info here, here, and also here)

Copy link
Collaborator

@ichard26 ichard26 left a comment

Choose a reason for hiding this comment

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

This might need a change log entry, but other than that LGTM, thanks!

edit: maybe when cache operations are done per run rather than per file in another PR, we can add the changelog entry along the lines of "Caching overhead when formatting large quantities of files has been significantly reduced" that covers both PRs including this one

edit 2: I can't read emails apparently, Black's caching DOES NOT behave how I described it as above ... today isn't a good day for me lol

@asottile
Copy link
Contributor Author

asottile commented Feb 4, 2021

added the changelog entry

@JelleZijlstra JelleZijlstra merged commit 3fca540 into psf:master Feb 4, 2021
@asottile asottile deleted the speed_up_cache branch February 4, 2021 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cache significantly slows down black due to pathlib
4 participants