-
Notifications
You must be signed in to change notification settings - Fork 4k
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 checksum computation at an collection-element level #75479
Conversation
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.
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.
I think it looks fine, but might want to run through speedometer to check
@@ -195,7 +195,16 @@ private static Checksum CreatePortableExecutableReferenceChecksum(PortableExecut | |||
using (var writer = new ObjectWriter(stream, leaveOpen: true)) | |||
{ | |||
WritePortableExecutableReferencePropertiesTo(reference, writer); | |||
WriteMvidsTo(TryGetMetadata(reference), writer); | |||
if (Path.IsPathRooted(reference.FilePath)) |
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 might want to run this through speedometer - it may flag new file access
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.
IsPathRooted is just a string operation, right?
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.
correct.
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.
Is this waiting on a speedometer run?
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.
yes.
Speedometer traces showed some expenses here. What can happen is you might have a
List of references
A that we compute a checksum against. But then if that list changes, but contains almost the entire set of references, we still recompute for the ones that are unchanged. This keeps around the checksums for the unchanged items.Also, we switch to consistently using a single mechanism to determine the mvid for PERef and ANalyzerReference. This avoids unnecessarily additional costs that exist to get teh full metadata info, that we don't need for checksum computation.
Build: https://devdiv.visualstudio.com/DefaultCollection/DevDiv/_build/results?buildId=10440954&view=results