-
Notifications
You must be signed in to change notification settings - Fork 206
improvement: Change the stamps to timewrapped ones #1871
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
Conversation
85c9962
to
3fd64a7
Compare
Previously we would use last modfied stamps for classfiles (aand other products) and hashes for jars. Now, we use hashes for both, but only if the modified time changed. Should help with scalameta/metals#4623
3fd64a7
to
0d4a9d4
Compare
Locally run benchmark seems to produce the same timings and I am running the server one to confirm. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, thank you!
I'll try that out and let you know how that may affect the performance on larger builds. Don't wait for me though 😄
fromBloopHashToZincHash(ByteHasher.hashFileContents(converter.toPath(file).toFile())) | ||
def forHash(fileRef: VirtualFileRef): Hash = { | ||
val file = converter.toPath(fileRef).toFile() | ||
if (file.exists()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to check if the file exists now but not before?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We only used that for libraries, so they usually never got deleted, but if they did we would throw an exception. I now use it for classfiles, so they might be deleted more often and it's expected.
Performance test finished successfully. Benchmarks is based on merging with master |
2 similar comments
Performance test finished successfully. Benchmarks is based on merging with master |
Performance test finished successfully. Benchmarks is based on merging with master |
Previously we would use last modfied stamps for classfiles (and other products) and hashes for jars. Now, we use hashes for both, but only if the modified time changed.
Should help with scalameta/metals#4623