-
Notifications
You must be signed in to change notification settings - Fork 1.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
Add more params to the evaluation pass stops #5978
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.
Awesome. This will help contextualize the elapsed-time numbers!
@@ -624,7 +624,7 @@ private void Evaluate() | |||
} | |||
|
|||
_data.InitialTargets = initialTargets; | |||
MSBuildEventSource.Log.EvaluatePass1Stop(projectFile); |
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 don't think this is accurate because _projectRootElement.Properties does not include properties of imported projects. See here and note that ProjectImportElements are not ProjectElementContainers.
Also, since it does a nontrivial amount of work, please surround it with an IsEnabled check. As-is, this will worsen solution load time.
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.
The ProjectImportGroupElement
is a ProjectElementContainer
and that should get included in both the Properties
recursion as it is with the PerformDepthFirstPass
recursion. Also the Count
is a property get on an already instantiated ReadOnlyCollection
(as a part of the actual evaluation above the log). No?
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 talked about this offline, but ProjectImportElement is not a ProjectElementContainer so isn't included, and accessing the Properties ReadOnlyCollection constructs it as needed, hence the perf hit.
No description provided.