Skip to content

Commit

Permalink
Support lazy instantiation of ProjectInstance values when CPS Evaluat…
Browse files Browse the repository at this point in the history
…ion cache is used (#10030)

### Context
This PR targets the specific scenario of constructing a ProjectInstance object from the evaluation data stored in CPS's evaluation cache, with the goal of saving memory and execution time during Solution Load.

In a previous change (2a789cd), I modified the ProjectInstance logic to rely on virtualized collections that are wrappers around the CPS evaluation cache's collection. The ProjectPropertyInstance, ProjectItemInstance, etc. objects were created ahead of time and associated with the corresponding CPS object, such that only one copy of the collections needed to exist.

In this PR, the goal is to extend that further such that the ProjectPropertyInstance, ProjectItemInstance, etc. values are only created when they're actually needed, and some new logic is introduced for reading Property values and Metadata values that allows avoiding creating the ProjectPropertyInstance and ProjectMetadataInstance when all that's sought is the EscapedValue string.

### Changes Made
The collections for ProjectInstance's _properties, _itemDefinitions, _items, _itemsByEvaluatedInclude, _globalProperties, _targets, _importPaths, and _importPathsincludingDuplicates collections are replaced with specialized virtualizing collections that wrap the associated CPS collection. These collections create the ProjectPropertyInstance et al objects only when a caller requests them. Additionally, the ProjectItemDefinitionInstance's metadata collection, and the ProjectItemInstance's ItemDefinition and Metadata collections are all similarly replaced by a virtualizing collection.

ProjectInstance's _environmentVariableProperties still relies on the ProjectCollection's EnvironmentVariables, but a new SharedReadOnlyEnvironmentProperties collection is introduced that does not hand out copies of the PropertyDictionary, but instead the same PropertyDictionary instance whose backing collection has been made read only.

### Testing
Manual verification of impacted scenarios and performance measurements.
  • Loading branch information
sgreenmsft authored May 24, 2024
1 parent 147ecad commit eade720
Show file tree
Hide file tree
Showing 29 changed files with 1,582 additions and 297 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ public void ImportProperties()
public void DeepClone()
{
CopyOnWritePropertyDictionary<MockValue> source = CreateInstance("a", "b", "c");
CopyOnWritePropertyDictionary<MockValue> clone = source.DeepClone();
CopyOnWritePropertyDictionary<MockValue> clone = (CopyOnWritePropertyDictionary<MockValue>)source.DeepClone();

source.ShouldBe(clone);
source.ShouldNotBeSameAs(clone);
Expand Down
2 changes: 1 addition & 1 deletion src/Build.UnitTests/Instance/TaskItem_Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,7 @@ public void Metadata()
item.MetadataCount.ShouldBe(s_builtInMetadataNames.Length + 2);
item.DirectMetadataCount.ShouldBe(1);

CopyOnWritePropertyDictionary<ProjectMetadataInstance> metadata = item.MetadataCollection;
ICopyOnWritePropertyDictionary<ProjectMetadataInstance> metadata = item.MetadataCollection;
metadata.Count.ShouldBe(2);
metadata["a"].EvaluatedValue.ShouldBe("override");
metadata["b"].EvaluatedValue.ShouldBe("base");
Expand Down
2 changes: 1 addition & 1 deletion src/Build/BackEnd/BuildManager/BuildParameters.cs
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ public class BuildParameters : ITranslatable
/// </summary>
public BuildParameters()
{
Initialize(Utilities.GetEnvironmentProperties(), new ProjectRootElementCache(false), null);
Initialize(Utilities.GetEnvironmentProperties(makeReadOnly: false), new ProjectRootElementCache(false), null);
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,8 @@ private List<ProjectItemInstance> ExpandItemIntoItems(
includeSplit /* before wildcard expansion */,
null,
null,
originalItem.Location.File));
originalItem.Location.File,
useItemDefinitionsWithoutModification: false));
}
}
}
Expand Down
80 changes: 70 additions & 10 deletions src/Build/Collections/CopyOnWritePropertyDictionary.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,39 +36,39 @@ namespace Microsoft.Build.Collections
/// </remarks>
/// <typeparam name="T">Property or Metadata class type to store</typeparam>
[DebuggerDisplay("#Entries={Count}")]
internal sealed class CopyOnWritePropertyDictionary<T> : IEnumerable<T>, IEquatable<CopyOnWritePropertyDictionary<T>>, IDictionary<string, T>
internal sealed class CopyOnWritePropertyDictionary<T> : ICopyOnWritePropertyDictionary<T>, IEquatable<CopyOnWritePropertyDictionary<T>>
where T : class, IKeyed, IValued, IEquatable<T>, IImmutable
{
private static readonly ImmutableDictionary<string, T> NameComparerDictionaryPrototype = ImmutableDictionary.Create<string, T>(MSBuildNameIgnoreCaseComparer.Default);

/// <summary>
/// Backing dictionary
/// Backing dictionary.
/// </summary>
private ImmutableDictionary<string, T> _backing;

/// <summary>
/// Creates empty dictionary
/// Creates empty dictionary.
/// </summary>
public CopyOnWritePropertyDictionary()
{
_backing = NameComparerDictionaryPrototype;
}

/// <summary>
/// Cloning constructor, with deferred cloning semantics
/// Cloning constructor, with deferred cloning semantics.
/// </summary>
private CopyOnWritePropertyDictionary(CopyOnWritePropertyDictionary<T> that)
{
_backing = that._backing;
}

/// <summary>
/// Accessor for the list of property names
/// Accessor for the list of property names.
/// </summary>
ICollection<string> IDictionary<string, T>.Keys => ((IDictionary<string, T>)_backing).Keys;

/// <summary>
/// Accessor for the list of properties
/// Accessor for the list of properties.
/// </summary>
ICollection<T> IDictionary<string, T>.Values => ((IDictionary<string, T>)_backing).Values;

Expand Down Expand Up @@ -115,6 +115,16 @@ public T this[string name]
/// </summary>
public bool Contains(string name) => _backing.ContainsKey(name);

public string GetEscapedValue(string name)
{
if (_backing.TryGetValue(name, out T value))
{
return value?.EscapedValue;
}

return null;
}

/// <summary>
/// Empties the collection
/// </summary>
Expand All @@ -134,7 +144,7 @@ public void Clear()
/// </summary>
IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();

#region IEquatable<PropertyDictionary<T>> Members
#region IEquatable<CopyOnWritePropertyDictionary<T>> Members

/// <summary>
/// Compares two property dictionaries for equivalence. They are equal if each contains the same properties with the
Expand Down Expand Up @@ -180,6 +190,56 @@ public bool Equals(CopyOnWritePropertyDictionary<T> other)

#endregion

#region IEquatable<CopyOnWritePropertyDictionary<T>> Members

/// <summary>
/// Compares two property dictionaries for equivalence. They are equal if each contains the same properties with the
/// same values as the other, unequal otherwise.
/// </summary>
/// <param name="other">The dictionary to which this should be compared</param>
/// <returns>True if they are equivalent, false otherwise.</returns>
public bool Equals(ICopyOnWritePropertyDictionary<T> other)
{
if (other == null)
{
return false;
}

ImmutableDictionary<string, T> thisBacking = _backing;
IDictionary<string, T> otherDict = other;

if (other is CopyOnWritePropertyDictionary<T> otherCopyOnWritePropertyDictionary)
{
// If the backing collections are the same, we are equal.
// Note that with this check, we intentionally avoid the common reference
// comparison between 'this' and 'other'.
if (ReferenceEquals(thisBacking, otherCopyOnWritePropertyDictionary._backing))
{
return true;
}

otherDict = otherCopyOnWritePropertyDictionary._backing;
}

if (thisBacking.Count != otherDict.Count)
{
return false;
}

foreach (T thisProp in thisBacking.Values)
{
if (!otherDict.TryGetValue(thisProp.Key, out T thatProp) ||
!EqualityComparer<T>.Default.Equals(thisProp, thatProp))
{
return false;
}
}

return true;
}

#endregion

#region IDictionary<string,T> Members

/// <summary>
Expand Down Expand Up @@ -274,7 +334,7 @@ public bool Remove(string name)
/// Overwrites any property with the same name already in the collection.
/// To remove a property, use Remove(...) instead.
/// </summary>
internal void Set(T projectProperty)
public void Set(T projectProperty)
{
ErrorUtilities.VerifyThrowArgumentNull(projectProperty, nameof(projectProperty));

Expand All @@ -285,7 +345,7 @@ internal void Set(T projectProperty)
/// Adds the specified properties to this dictionary.
/// </summary>
/// <param name="other">An enumerator over the properties to add.</param>
internal void ImportProperties(IEnumerable<T> other)
public void ImportProperties(IEnumerable<T> other)
{
_backing = _backing.SetItems(Items());

Expand All @@ -302,7 +362,7 @@ IEnumerable<KeyValuePair<string, T>> Items()
/// Clone. As we're copy on write, this
/// should be cheap.
/// </summary>
internal CopyOnWritePropertyDictionary<T> DeepClone()
public ICopyOnWritePropertyDictionary<T> DeepClone()
{
return new CopyOnWritePropertyDictionary<T>(this);
}
Expand Down
65 changes: 65 additions & 0 deletions src/Build/Collections/ICopyOnWritePropertyDictionary.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using Microsoft.Build.Shared;

#nullable disable

namespace Microsoft.Build.Collections
{
/// <summary>
/// An interface that represents a dictionary of unordered property or metadata name/value pairs with copy-on-write semantics.
/// </summary>
/// <remarks>
/// The value that this adds over IDictionary&lt;string, T&gt; is:
/// - supports copy on write
/// - enforces that key = T.Name
/// - default enumerator is over values
/// - (marginal) enforces the correct key comparer
///
/// Really a Dictionary&lt;string, T&gt; where the key (the name) is obtained from IKeyed.Key.
/// Is not observable, so if clients wish to observe modifications they must mediate them themselves and
/// either not expose this collection or expose it through a readonly wrapper.
///
/// This collection is safe for concurrent readers and a single writer.
/// </remarks>
/// <typeparam name="T">Property or Metadata class type to store</typeparam>
internal interface ICopyOnWritePropertyDictionary<T> : IEnumerable<T>, IEquatable<ICopyOnWritePropertyDictionary<T>>, IDictionary<string, T>
where T : class, IKeyed, IValued, IEquatable<T>, IImmutable
{
/// <summary>
/// Returns true if a property with the specified name is present in the collection, otherwise false.
/// </summary>
bool Contains(string name);

/// <summary>
/// Add the specified property to the collection.
/// Overwrites any property with the same name already in the collection.
/// To remove a property, use Remove(...) instead.
/// </summary>
void Set(T projectProperty);

/// <summary>
/// Adds the specified properties to this dictionary.
/// </summary>
/// <param name="other">An enumerator over the properties to add.</param>
void ImportProperties(IEnumerable<T> other);

/// <summary>
/// Clone. As we're copy on write, this should be cheap.
/// </summary>
ICopyOnWritePropertyDictionary<T> DeepClone();

/// <summary>
/// <typeparamref name="T"/> must implement <see cref="IKeyed"/>, which means it contains an
/// EscapedValue. This method allows retrieving the EscapedValue of an object in the dictionary
/// directly.
/// </summary>
string GetEscapedValue(string name);
}
}
24 changes: 24 additions & 0 deletions src/Build/Collections/IMultiDictionary.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace Microsoft.Build.Collections
{
/// <summary>
/// Represents a dictionary that can hold more than one distinct value with the same key.
/// All keys must have at least one value: null values are currently rejected.
/// </summary>
/// <typeparam name="K">Type of key</typeparam>
/// <typeparam name="V">Type of value</typeparam>
internal interface IMultiDictionary<K, V>
where K : class
where V : class
{
IEnumerable<V> this[K key] { get; }
}
}
4 changes: 2 additions & 2 deletions src/Build/Collections/MultiDictionary.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ namespace Microsoft.Build.Collections
/// <typeparam name="K">Type of key</typeparam>
/// <typeparam name="V">Type of value</typeparam>
[DebuggerDisplay("#Keys={KeyCount} #Values={ValueCount}")]
internal class MultiDictionary<K, V>
internal class MultiDictionary<K, V> : IMultiDictionary<K, V>
where K : class
where V : class
{
Expand Down Expand Up @@ -86,7 +86,7 @@ internal MultiDictionary(IEqualityComparer<K> keyComparer)
/// <summary>
/// Enumerator over values that have the specified key.
/// </summary>
internal IEnumerable<V> this[K key]
public IEnumerable<V> this[K key]
{
get
{
Expand Down
28 changes: 23 additions & 5 deletions src/Build/Collections/PropertyDictionary.cs
Original file line number Diff line number Diff line change
Expand Up @@ -42,22 +42,22 @@ internal sealed class PropertyDictionary<T> : IEnumerable<T>, IEquatable<Propert
/// Backing dictionary
/// </summary>
[DebuggerBrowsable(DebuggerBrowsableState.RootHidden)]
private readonly IRetrievableEntryHashSet<T> _properties;
private readonly IRetrievableValuedEntryHashSet<T> _properties;

/// <summary>
/// Creates empty dictionary
/// </summary>
public PropertyDictionary()
{
_properties = new RetrievableEntryHashSet<T>(MSBuildNameIgnoreCaseComparer.Default);
_properties = new RetrievableValuedEntryHashSet<T>(MSBuildNameIgnoreCaseComparer.Default);
}

/// <summary>
/// Creates empty dictionary, optionally specifying initial capacity
/// </summary>
internal PropertyDictionary(int capacity)
{
_properties = new RetrievableEntryHashSet<T>(capacity, MSBuildNameIgnoreCaseComparer.Default);
_properties = new RetrievableValuedEntryHashSet<T>(capacity, MSBuildNameIgnoreCaseComparer.Default);
}

/// <summary>
Expand All @@ -77,7 +77,7 @@ internal PropertyDictionary(IEnumerable<T> elements)
/// </summary>
internal PropertyDictionary(MSBuildNameIgnoreCaseComparer comparer)
{
_properties = new RetrievableEntryHashSet<T>(comparer);
_properties = new RetrievableValuedEntryHashSet<T>(comparer);
}

/// <summary>
Expand All @@ -96,7 +96,7 @@ internal PropertyDictionary(int capacity, IEnumerable<T> elements)
/// Initializes a new instance of the <see cref="PropertyDictionary{T}"/> class.
/// </summary>
/// <param name="propertiesHashSet">The collection of properties to use.</param>
internal PropertyDictionary(IRetrievableEntryHashSet<T> propertiesHashSet)
internal PropertyDictionary(IRetrievableValuedEntryHashSet<T> propertiesHashSet)
{
_properties = propertiesHashSet;
}
Expand Down Expand Up @@ -324,6 +324,24 @@ public T Get(string keyString, int startIndex, int endIndex)
return GetProperty(keyString, startIndex, endIndex);
}

/// <summary>
/// Gets the unescaped value of a particular property.
/// </summary>
/// <param name="propertyName">The name of the property whose value is sought.</param>
/// <param name="unescapedValue">The out parameter by which a successfully retrieved value is returned.</param>
/// <returns>True if a property with a matching name was found. False otherwise.</returns>
public bool TryGetPropertyUnescapedValue(string propertyName, out string unescapedValue)
{
if (_properties.TryGetEscapedValue(propertyName, out string escapedValue) && escapedValue != null)
{
unescapedValue = EscapingUtilities.UnescapeAll(escapedValue);
return true;
}

unescapedValue = null;
return false;
}

#region IDictionary<string,T> Members

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@

namespace Microsoft.Build.Collections
{
/// <summary>
/// Represents a hash set mapping string to <typeparamref name="T"/>, with the specialization that
/// value lookup supports using substrings of a provided key without requiring instantiating the substring
/// (in order to avoid the memory usage of string allocation).
/// </summary>
/// <typeparam name="T">The type of data the hash set contains (which must be <see cref="IKeyed"/>).</typeparam>
internal interface IRetrievableEntryHashSet<T> :
ICollection<T>,
ISerializable,
Expand Down
Loading

0 comments on commit eade720

Please sign in to comment.