-
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
Avoid unnecessary array+linq allocs in common case #73727
Conversation
src/Workspaces/Core/Portable/Workspace/Solution/SolutionCompilationState.cs
Outdated
Show resolved
Hide resolved
// the same text (for example, when GetOpenDocumentInCurrentContextWithChanges) is called. | ||
// | ||
// The use of GetRequiredState mirrors what happens in WithDocumentTexts | ||
if (!SourceTextIsUnchanged(documentState, text)) |
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.
Yes. That definitely makes sense. I can tweak to that when I get home. Lmk if this is otherwise ok
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.
done.
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.
Addresses large allocation here:
The issue is not the array allocation itself, it's that we're doing anything allocating for a very common case where we're called to update a document with the text it already points at. This can happen during things like GetOpenDocumentWithChanges:
In hte majority of cases, the doc will be updated to the content it already has. So bailing out immediately, without doing any work is beneficial.