Skip to content

Commit

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

This reverts commit eade720.
  • Loading branch information
sgreenmsft authored Jun 4, 2024
1 parent 4b522b0 commit 9bea802
Show file tree
Hide file tree
Showing 29 changed files with 297 additions and 1,582 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 = (CopyOnWritePropertyDictionary<MockValue>)source.DeepClone();
CopyOnWritePropertyDictionary<MockValue> clone = 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);

ICopyOnWritePropertyDictionary<ProjectMetadataInstance> metadata = item.MetadataCollection;
CopyOnWritePropertyDictionary<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(makeReadOnly: false), new ProjectRootElementCache(false), null);
Initialize(Utilities.GetEnvironmentProperties(), new ProjectRootElementCache(false), null);
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -457,8 +457,7 @@ private List<ProjectItemInstance> ExpandItemIntoItems(
includeSplit /* before wildcard expansion */,
null,
null,
originalItem.Location.File,
useItemDefinitionsWithoutModification: false));
originalItem.Location.File));
}
}
}
Expand Down
80 changes: 10 additions & 70 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> : ICopyOnWritePropertyDictionary<T>, IEquatable<CopyOnWritePropertyDictionary<T>>
internal sealed class CopyOnWritePropertyDictionary<T> : IEnumerable<T>, IEquatable<CopyOnWritePropertyDictionary<T>>, IDictionary<string, 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,16 +115,6 @@ 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 @@ -144,7 +134,7 @@ public void Clear()
/// </summary>
IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();

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

/// <summary>
/// Compares two property dictionaries for equivalence. They are equal if each contains the same properties with the
Expand Down Expand Up @@ -190,56 +180,6 @@ 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 @@ -334,7 +274,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>
public void Set(T projectProperty)
internal void Set(T projectProperty)
{
ErrorUtilities.VerifyThrowArgumentNull(projectProperty, nameof(projectProperty));

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

Expand All @@ -362,7 +302,7 @@ IEnumerable<KeyValuePair<string, T>> Items()
/// Clone. As we're copy on write, this
/// should be cheap.
/// </summary>
public ICopyOnWritePropertyDictionary<T> DeepClone()
internal CopyOnWritePropertyDictionary<T> DeepClone()
{
return new CopyOnWritePropertyDictionary<T>(this);
}
Expand Down
65 changes: 0 additions & 65 deletions src/Build/Collections/ICopyOnWritePropertyDictionary.cs

This file was deleted.

24 changes: 0 additions & 24 deletions src/Build/Collections/IMultiDictionary.cs

This file was deleted.

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> : IMultiDictionary<K, V>
internal class MultiDictionary<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>
public IEnumerable<V> this[K key]
internal IEnumerable<V> this[K key]
{
get
{
Expand Down
28 changes: 5 additions & 23 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 IRetrievableValuedEntryHashSet<T> _properties;
private readonly IRetrievableEntryHashSet<T> _properties;

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

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

/// <summary>
Expand All @@ -77,7 +77,7 @@ internal PropertyDictionary(IEnumerable<T> elements)
/// </summary>
internal PropertyDictionary(MSBuildNameIgnoreCaseComparer comparer)
{
_properties = new RetrievableValuedEntryHashSet<T>(comparer);
_properties = new RetrievableEntryHashSet<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(IRetrievableValuedEntryHashSet<T> propertiesHashSet)
internal PropertyDictionary(IRetrievableEntryHashSet<T> propertiesHashSet)
{
_properties = propertiesHashSet;
}
Expand Down Expand Up @@ -324,24 +324,6 @@ 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 @@ -320,7 +320,7 @@ public bool TryGetValue(string key, out T item)
/// Gets the item if any with the given name
/// </summary>
/// <param name="key">key to check for containment</param>
/// <returns>The item, if it was found. Otherwise, default(T).</returns>
/// <returns>true if item contained; false if not</returns>
public T Get(string key)
{
return GetCore(key, 0, key?.Length ?? 0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,6 @@

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 9bea802

Please sign in to comment.