From 944145c64cb9951dbcd2d65314889d7f06275536 Mon Sep 17 00:00:00 2001 From: Mihai Codoban Date: Wed, 21 Sep 2016 15:39:23 -0700 Subject: [PATCH] Only record extra Evaluator data when needed (#1060) This saves memory --- src/XMakeBuildEngine/Definition/Project.cs | 86 ++++++++++++++----- src/XMakeBuildEngine/Evaluation/Evaluator.cs | 14 ++- .../Definition/Project_Tests.cs | 35 ++++++++ 3 files changed, 110 insertions(+), 25 deletions(-) diff --git a/src/XMakeBuildEngine/Definition/Project.cs b/src/XMakeBuildEngine/Definition/Project.cs index 51f3fef3720..b4fea486dcf 100644 --- a/src/XMakeBuildEngine/Definition/Project.cs +++ b/src/XMakeBuildEngine/Definition/Project.cs @@ -61,7 +61,12 @@ public enum ProjectLoadSettings /// /// Throw an exception and stop the evaluation of a project if any circular imports are detected /// - RejectCircularImports = 4 + RejectCircularImports = 4, + + /// + /// Record the item elements that got evaluated + /// + RecordEvaluatedItemElements = 8 } /// @@ -1069,6 +1074,7 @@ public static string GetEvaluatedItemIncludeEscaped(ProjectItemDefinition item) /// public List GetAllGlobs() { + ReevaluateForPostMortemAnalysisIfNecessary(); return GetAllGlobs(_data.EvaluatedItemElements); } @@ -1082,6 +1088,8 @@ public List GetAllGlobs(string itemType) { return new List(); } + + ReevaluateForPostMortemAnalysisIfNecessary(); return GetAllGlobs(GetItemElementsByType(_data.EvaluatedItemElements, itemType)); } @@ -1156,6 +1164,7 @@ private IEnumerable GetAllGlobs(ProjectItemElement itemElement) /// public List GetItemProvenance(string itemToMatch) { + ReevaluateForPostMortemAnalysisIfNecessary(); return GetItemProvenance(itemToMatch, _data.EvaluatedItemElements); } @@ -1166,6 +1175,7 @@ public List GetItemProvenance(string itemToMatch) /// The item type to constrain the search in public List GetItemProvenance(string itemToMatch, string itemType) { + ReevaluateForPostMortemAnalysisIfNecessary(); return GetItemProvenance(itemToMatch, GetItemElementsByType(_data.EvaluatedItemElements, itemType)); } @@ -1184,11 +1194,30 @@ public List GetItemProvenance(ProjectItem item) return new List(); } + ReevaluateForPostMortemAnalysisIfNecessary(); var itemElementsAbove = GetItemElementsThatMightAffectItem(_data.EvaluatedItemElements, item); return GetItemProvenance(item.EvaluatedInclude, itemElementsAbove); } + /// + /// Some project APIs need to do analysis that requires the Evaluator to record more data than usual as it evaluates. + /// This method checks if the Evaluator was run with the extra required settings and if not, does a re-evaluation. + /// If a re-evaluation was necessary, it saves this information so a next call does not re-evaluate. + /// + /// Using this method avoids storing extra data in memory when its not needed. + /// + private void ReevaluateForPostMortemAnalysisIfNecessary() + { + if (_loadSettings.HasFlag(ProjectLoadSettings.RecordEvaluatedItemElements)) + { + return; + } + + _loadSettings = _loadSettings | ProjectLoadSettings.RecordEvaluatedItemElements; + Reevaluate(LoggingService, _loadSettings); + } + private static IEnumerable GetItemElementsThatMightAffectItem(List evaluatedItemElements, ProjectItem item) { return evaluatedItemElements @@ -2392,6 +2421,14 @@ private ProjectInstance CreateProjectInstance(ILoggingService loggingServiceForE /// Re-evaluates the project using the specified logging service. /// private void ReevaluateIfNecessary(ILoggingService loggingServiceForEvaluation) + { + ReevaluateIfNecessary(loggingServiceForEvaluation, _loadSettings); + } + + /// + /// Re-evaluates the project using the specified logging service and load settings. + /// + private void ReevaluateIfNecessary(ILoggingService loggingServiceForEvaluation, ProjectLoadSettings loadSettings) { // We will skip the evaluation if the flag is set. This will give us better performance on scenarios // that we know we don't have to reevaluate. One example is project conversion bulk addfiles and set attributes. @@ -2399,27 +2436,7 @@ private void ReevaluateIfNecessary(ILoggingService loggingServiceForEvaluation) { try { - Evaluator.Evaluate(_data, _xml, _loadSettings, ProjectCollection.MaxNodeCount, ProjectCollection.EnvironmentProperties, loggingServiceForEvaluation, new ProjectItemFactory(this), _projectCollection as IToolsetProvider, _projectCollection.ProjectRootElementCache, s_buildEventContext, null /* no project instance for debugging */); - - // We have to do this after evaluation, because evaluation might have changed - // the imports being pulled in. - int highestXmlVersion = Xml.Version; - - if (_data.ImportClosure != null) - { - foreach (Triple triple in _data.ImportClosure) - { - highestXmlVersion = (highestXmlVersion < triple.Third) ? triple.Third : highestXmlVersion; - } - } - - _explicitlyMarkedDirty = false; - _evaluatedVersion = highestXmlVersion; - _evaluatedToolsetCollectionVersion = ProjectCollection.ToolsetsVersion; - _evaluationCounter = GetNextEvaluationCounter(); - _data.HasUnsavedChanges = false; - - ErrorUtilities.VerifyThrow(!IsDirty, "Should not be dirty now"); + Reevaluate(loggingServiceForEvaluation, loadSettings); } catch (InvalidProjectFileException ex) { @@ -2429,6 +2446,31 @@ private void ReevaluateIfNecessary(ILoggingService loggingServiceForEvaluation) } } + private void Reevaluate(ILoggingService loggingServiceForEvaluation, ProjectLoadSettings loadSettings) + { + Evaluator.Evaluate(_data, _xml, loadSettings, ProjectCollection.MaxNodeCount, ProjectCollection.EnvironmentProperties, loggingServiceForEvaluation, new ProjectItemFactory(this), _projectCollection as IToolsetProvider, _projectCollection.ProjectRootElementCache, s_buildEventContext, null /* no project instance for debugging */); + + // We have to do this after evaluation, because evaluation might have changed + // the imports being pulled in. + int highestXmlVersion = Xml.Version; + + if (_data.ImportClosure != null) + { + foreach (Triple triple in _data.ImportClosure) + { + highestXmlVersion = (highestXmlVersion < triple.Third) ? triple.Third : highestXmlVersion; + } + } + + _explicitlyMarkedDirty = false; + _evaluatedVersion = highestXmlVersion; + _evaluatedToolsetCollectionVersion = ProjectCollection.ToolsetsVersion; + _evaluationCounter = GetNextEvaluationCounter(); + _data.HasUnsavedChanges = false; + + ErrorUtilities.VerifyThrow(!IsDirty, "Should not be dirty now"); + } + /// /// Common code for the constructors. /// Applies global properties that are on the collection. diff --git a/src/XMakeBuildEngine/Evaluation/Evaluator.cs b/src/XMakeBuildEngine/Evaluation/Evaluator.cs index 40543448b2b..f3cfc7cddc4 100644 --- a/src/XMakeBuildEngine/Evaluation/Evaluator.cs +++ b/src/XMakeBuildEngine/Evaluation/Evaluator.cs @@ -1642,7 +1642,7 @@ private void EvaluateItemElement(bool itemGroupConditionResult, ProjectItemEleme if (conditionResult) { - _data.EvaluatedItemElements.Add(itemElement); + RecordEvaluatedItemElement(itemElement); } return; @@ -1660,7 +1660,7 @@ private void EvaluateItemElement(bool itemGroupConditionResult, ProjectItemEleme private void EvaluateItemElementUpdate(ProjectItemElement itemElement) { - _data.EvaluatedItemElements.Add(itemElement); + RecordEvaluatedItemElement(itemElement); var expandedItemSet = new HashSet( @@ -1734,7 +1734,7 @@ private void EvaluateItemElementInclude(bool itemGroupConditionResult, bool item // FINALLY: Add the items to the project if (itemConditionResult && itemGroupConditionResult) { - _data.EvaluatedItemElements.Add(itemElement); + RecordEvaluatedItemElement(itemElement); foreach (I item in items) { @@ -2624,6 +2624,14 @@ private string GetCurrentDirectoryForConditionEvaluation(ProjectElement element) } } + private void RecordEvaluatedItemElement(ProjectItemElement itemElement) + { + if (_loadSettings.HasFlag(ProjectLoadSettings.RecordEvaluatedItemElements)) + { + _data.EvaluatedItemElements.Add(itemElement); + } + } + /// /// Throws InvalidProjectException because we failed to import a project which contained a ProjectImportSearchPath fall-back. /// MSBuildExtensionsPath reference kind found in the Project attribute of the Import element diff --git a/src/XMakeBuildEngine/UnitTestsPublicOM/Definition/Project_Tests.cs b/src/XMakeBuildEngine/UnitTestsPublicOM/Definition/Project_Tests.cs index bcc311add1a..ecc2e72449b 100644 --- a/src/XMakeBuildEngine/UnitTestsPublicOM/Definition/Project_Tests.cs +++ b/src/XMakeBuildEngine/UnitTestsPublicOM/Definition/Project_Tests.cs @@ -2535,6 +2535,41 @@ public void GetItemProvenanceOnlyGlob() AssertProvenanceResult(expected, project, "2.foo"); } + [Fact] + public void GetItemProvenanceShouldReturnTheSameResultsIfProjectIsReevaluated() + { + var projectContents = + @" + + + + + + + "; + + var expected = new ProvenanceResultTupleList + { + Tuple.Create("A", Operation.Include, Provenance.Glob, 1), + Tuple.Create("B", Operation.Exclude, Provenance.Glob, 1) + }; + + // Create a project. The initial evaluation does not record the information needed for GetItemProvenance + var project = ObjectModelHelpers.CreateInMemoryProject(projectContents); + + // Since GetItemProvenance does not have the required evaluator data (evaluated item elements), it internally reevaluates the project to collect it + var provenanceResult = project.GetItemProvenance("2.foo"); + AssertProvenanceResult(expected, provenanceResult); + + // Dirty the xml to force another reevaluation. + project.AddItem("new", "new value"); + project.ReevaluateIfNecessary(); + + // Assert that provenance returns the same result and that no data duplication happened + provenanceResult = project.GetItemProvenance("2.foo"); + AssertProvenanceResult(expected, provenanceResult); + } + [Fact] public void GetItemProvenanceShouldHandleComplexGlobExclusion() {