Skip to content

Commit

Permalink
Optimize RAR's GetReferenceItems method (#5929)
Browse files Browse the repository at this point in the history 
* Iterate over dictionary once

* Avoid unnecessary initializations

* Set copy local more efficiently

* referenceItem should have no metadata initially

* Update larger metadata dictionary

* Set fusionName at end

* Slightly simplify referenceItem creation

* Fix typos

* Addition to setting fusionName at end

* Shift other types of removed metadata later so they only have to be removed once

nonForwardableMetadata?.Remove(ItemMetadataNames.*) is used to prevent the correct metadata value from being overwritten when metadata are re-added from nonForwardableMetadata.

* Use ternary operators

* Set items as a group
  • Loading branch information
@Forgind
Forgind committed Feb 6, 2021
1 parent 08fae4e commit 8fb627e
Show file tree
Hide file tree
Showing 4 changed files with 145 additions and 110 deletions.
11 changes: 11 additions & 0 deletions src/MSBuildTaskHost/Immutable/ImmutableDictionary.cs
View file
Expand Up @@ -138,6 +138,17 @@ private void CheckCopyToArguments(Array array, int arrayIndex)
return clone;
}

internal ImmutableDictionary<K, V> SetItems(IEnumerable<KeyValuePair<K, V>> items)
{
var clone = new ImmutableDictionary<K, V>(_backing);
foreach (KeyValuePair<K, V> item in items)
{
clone._backing[item.Key] = item.Value;
}

return clone;
}

internal ImmutableDictionary<K, V> Remove(K key)
{
if (!ContainsKey(key))
Expand Down
12 changes: 12 additions & 0 deletions src/Shared/CopyOnWriteDictionary.cs
View file
Expand Up @@ -227,6 +227,18 @@ public void Add(string key, V value)
_backing = _backing.SetItem(key, value);
}

/// <summary>
/// Adds several value to the dictionary.
/// </summary>
public void SetItems(IEnumerable<KeyValuePair<string, V>> items)
{
_backing = _backing.SetItems(items);
}

public IEnumerable<KeyValuePair<string, V>> Where(Func<KeyValuePair<string, V>, bool> predicate)
{
return _backing.Where(predicate);
}
/// <summary>
/// Returns true if the dictionary contains the specified key.
/// </summary>
Expand Down
209 changes: 103 additions & 106 deletions src/Tasks/AssemblyDependency/ReferenceTable.cs
View file
Expand Up @@ -2616,14 +2616,6 @@ internal void GetReferenceItems
out ITaskItem[] copyLocalFiles
)
{
primaryFiles = Array.Empty<ITaskItem>();
dependencyFiles = Array.Empty<ITaskItem>();
relatedFiles = Array.Empty<ITaskItem>();
satelliteFiles = Array.Empty<ITaskItem>();
serializationAssemblyFiles = Array.Empty<ITaskItem>();
scatterFiles = Array.Empty<ITaskItem>();
copyLocalFiles = Array.Empty<ITaskItem>();

var primaryItems = new List<ITaskItem>();
var dependencyItems = new List<ITaskItem>();
var relatedItems = new List<ITaskItem>();
Expand All @@ -2632,10 +2624,10 @@ internal void GetReferenceItems
var scatterItems = new List<ITaskItem>();
var copyLocalItems = new List<ITaskItem>();

foreach (AssemblyNameExtension assemblyName in References.Keys)
foreach (KeyValuePair<AssemblyNameExtension, Reference> kvp in References)
{
string fusionName = assemblyName.FullName;
Reference reference = GetReference(assemblyName);
AssemblyNameExtension assemblyName = kvp.Key;
Reference reference = kvp.Value;

// Conflict victims and badimages are filtered out.
if (!reference.IsBadImage)
Expand Down Expand Up @@ -2664,7 +2656,7 @@ internal void GetReferenceItems

if (reference.IsResolved)
{
ITaskItem referenceItem = SetItemMetadata(relatedItems, satelliteItems, serializationAssemblyItems, scatterItems, fusionName, reference, assemblyName);
ITaskItem referenceItem = SetItemMetadata(relatedItems, satelliteItems, serializationAssemblyItems, scatterItems, assemblyName.FullName, reference, assemblyName);

if (reference.IsPrimary)
{
Expand All @@ -2683,9 +2675,7 @@ internal void GetReferenceItems
}
}

primaryFiles = new ITaskItem[primaryItems.Count];
primaryItems.CopyTo(primaryFiles, 0);

primaryFiles = primaryItems.ToArray();
dependencyFiles = dependencyItems.ToArray();
relatedFiles = relatedItems.ToArray();
satelliteFiles = satelliteItems.ToArray();
Expand All @@ -2711,22 +2701,12 @@ internal void GetReferenceItems
private ITaskItem SetItemMetadata(List<ITaskItem> relatedItems, List<ITaskItem> satelliteItems, List<ITaskItem> serializationAssemblyItems, List<ITaskItem> scatterItems, string fusionName, Reference reference, AssemblyNameExtension assemblyName)
{
// Set up the main item.
ITaskItem referenceItem = new TaskItem();
TaskItem referenceItem = new TaskItem();
referenceItem.ItemSpec = reference.FullPath;
referenceItem.SetMetadata(ItemMetadataNames.resolvedFrom, reference.ResolvedSearchPath);

// Set the CopyLocal metadata.
if (reference.IsCopyLocal)
{
referenceItem.SetMetadata(ItemMetadataNames.copyLocal, "true");
}
else
{
referenceItem.SetMetadata(ItemMetadataNames.copyLocal, "false");
}

// Set the FusionName metadata.
referenceItem.SetMetadata(ItemMetadataNames.fusionName, fusionName);
referenceItem.SetMetadata(ItemMetadataNames.copyLocal, reference.IsCopyLocal ? "true" : "false");

// Set the Redist name metadata.
if (!String.IsNullOrEmpty(reference.RedistName))
Expand All @@ -2747,57 +2727,11 @@ private ITaskItem SetItemMetadata(List<ITaskItem> relatedItems, List<ITaskItem>
referenceItem.SetMetadata(ItemMetadataNames.imageRuntime, reference.ImageRuntime);
}

if (reference.IsWinMDFile)
{
referenceItem.SetMetadata(ItemMetadataNames.winMDFile, "true");

// The ImplementationAssembly is only set if the implementation file exits on disk
if (reference.ImplementationAssembly != null)
{
if (VerifyArchitectureOfImplementationDll(reference.ImplementationAssembly, reference.FullPath))
{
if (string.IsNullOrEmpty(referenceItem.GetMetadata(ItemMetadataNames.winmdImplmentationFile)))
{
referenceItem.SetMetadata(ItemMetadataNames.winmdImplmentationFile, Path.GetFileName(reference.ImplementationAssembly));
}

// Add the implementation item as a related file
ITaskItem item = new TaskItem(reference.ImplementationAssembly);
// Clone metadata.
referenceItem.CopyMetadataTo(item);
// Related files don't have a fusion name.
item.SetMetadata(ItemMetadataNames.fusionName, "");
RemoveNonForwardableMetadata(item);

// Add the related item.
relatedItems.Add(item);
}
}

if (reference.IsManagedWinMDFile)
{
referenceItem.SetMetadata(ItemMetadataNames.winMDFileType, "Managed");
}
else
{
referenceItem.SetMetadata(ItemMetadataNames.winMDFileType, "Native");
}
}

// Set the IsRedistRoot metadata
if (reference.IsRedistRoot == true)
{
referenceItem.SetMetadata(ItemMetadataNames.isRedistRoot, "true");
}
else if (reference.IsRedistRoot == false)
{
referenceItem.SetMetadata(ItemMetadataNames.isRedistRoot, "false");
}
else
// The redist root is "null" when there was no IsRedistRoot flag in the Redist XML
// (or there was no redist XML at all for this item).
if (reference.IsRedistRoot != null)
{
// This happens when the redist root is "null". This means there
// was no IsRedistRoot flag in the Redist XML (or there was no
// redist XML at all for this item).
referenceItem.SetMetadata(ItemMetadataNames.isRedistRoot, (bool)reference.IsRedistRoot ? "true" : "false");
}

// If there was a primary source item, then forward metadata from it.
Expand Down Expand Up @@ -2826,14 +2760,14 @@ private ITaskItem SetItemMetadata(List<ITaskItem> relatedItems, List<ITaskItem>
}

// If the item originally did not have the implementation file metadata then we do not want to get it from the set of primary source items
// since the implementation file is something specific to the source item and not supposed to be propigated.
// since the implementation file is something specific to the source item and not supposed to be propagated.
if (!hasImplementationFile)
{
referenceItem.RemoveMetadata(ItemMetadataNames.winmdImplmentationFile);
}

// If the item originally did not have the ImageRuntime metadata then we do not want to get it from the set of primary source items
// since the ImageRuntime is something specific to the source item and not supposed to be propigated.
// since the ImageRuntime is something specific to the source item and not supposed to be propagated.
if (!hasImageRuntime)
{
referenceItem.RemoveMetadata(ItemMetadataNames.imageRuntime);
Expand All @@ -2847,68 +2781,63 @@ private ITaskItem SetItemMetadata(List<ITaskItem> relatedItems, List<ITaskItem>
}
}

if (reference.ReferenceVersion != null)
{
referenceItem.SetMetadata(ItemMetadataNames.version, reference.ReferenceVersion.ToString());
}
else
referenceItem.SetMetadata(ItemMetadataNames.version, reference.ReferenceVersion == null ? string.Empty : reference.ReferenceVersion.ToString());

// Unset fusionName so we don't have to unset it later.
referenceItem.RemoveMetadata(ItemMetadataNames.fusionName);

List<string> relatedFileExtensions = reference.GetRelatedFileExtensions();
List<string> satellites = reference.GetSatelliteFiles();
List<string> serializationAssemblyFiles = reference.GetSerializationAssemblyFiles();
string[] scatterFiles = reference.GetScatterFiles();
Dictionary<string, string> nonForwardableMetadata = null;
if (relatedFileExtensions.Count > 0 || satellites.Count > 0 || serializationAssemblyFiles.Count > 0 || scatterFiles.Length > 0)
{
referenceItem.SetMetadata(ItemMetadataNames.version, String.Empty);
// Unset non-forwardable metadata now so we don't have to do it for individual items.
nonForwardableMetadata = RemoveNonForwardableMetadata(referenceItem);
}

// Now clone all properties onto the related files.
foreach (string relatedFileExtension in reference.GetRelatedFileExtensions())
foreach (string relatedFileExtension in relatedFileExtensions)
{
ITaskItem item = new TaskItem(reference.FullPathWithoutExtension + relatedFileExtension);
// Clone metadata.
referenceItem.CopyMetadataTo(item);
// Related files don't have a fusion name.
item.SetMetadata(ItemMetadataNames.fusionName, "");
RemoveNonForwardableMetadata(item);

// Add the related item.
relatedItems.Add(item);
}

// Set up the satellites.
foreach (string satelliteFile in reference.GetSatelliteFiles())
foreach (string satelliteFile in satellites)
{
ITaskItem item = new TaskItem(Path.Combine(reference.DirectoryName, satelliteFile));
// Clone metadata.
referenceItem.CopyMetadataTo(item);
// Set the destination directory.
item.SetMetadata(ItemMetadataNames.destinationSubDirectory, FileUtilities.EnsureTrailingSlash(Path.GetDirectoryName(satelliteFile)));
// Satellite files don't have a fusion name.
item.SetMetadata(ItemMetadataNames.fusionName, "");
RemoveNonForwardableMetadata(item);

// Add the satellite item.
satelliteItems.Add(item);
}

// Set up the serialization assemblies
foreach (string serializationAssemblyFile in reference.GetSerializationAssemblyFiles())
foreach (string serializationAssemblyFile in serializationAssemblyFiles)
{
ITaskItem item = new TaskItem(Path.Combine(reference.DirectoryName, serializationAssemblyFile));
// Clone metadata.
referenceItem.CopyMetadataTo(item);
// serialization assemblies files don't have a fusion name.
item.SetMetadata(ItemMetadataNames.fusionName, "");
RemoveNonForwardableMetadata(item);

// Add the serialization assembly item.
serializationAssemblyItems.Add(item);
}

// Set up the scatter files.
foreach (string scatterFile in reference.GetScatterFiles())
foreach (string scatterFile in scatterFiles)
{
ITaskItem item = new TaskItem(Path.Combine(reference.DirectoryName, scatterFile));
// Clone metadata.
referenceItem.CopyMetadataTo(item);
// We don't have a fusion name for scatter files.
item.SetMetadata(ItemMetadataNames.fusionName, "");
RemoveNonForwardableMetadata(item);

// Add the satellite item.
scatterItems.Add(item);
Expand All @@ -2928,6 +2857,61 @@ private ITaskItem SetItemMetadata(List<ITaskItem> relatedItems, List<ITaskItem>
}
}

if (reference.IsWinMDFile)
{
// The ImplementationAssembly is only set if the implementation file exits on disk
if (reference.ImplementationAssembly != null)
{
if (VerifyArchitectureOfImplementationDll(reference.ImplementationAssembly, reference.FullPath))
{
// Add the implementation item as a related file
ITaskItem item = new TaskItem(reference.ImplementationAssembly);
// Clone metadata.
referenceItem.CopyMetadataTo(item);

// Add the related item.
relatedItems.Add(item);

referenceItem.SetMetadata(ItemMetadataNames.winmdImplmentationFile, Path.GetFileName(reference.ImplementationAssembly));
// This may have been set previously (before it was removed so we could more efficiently set metadata on the various related files).
// This version should take priority, so we remove it from nonForwardableMetadata if it's there to prevent the correct value from
// being overwritten.
nonForwardableMetadata?.Remove(ItemMetadataNames.winmdImplmentationFile);
}
}

// This may have been set previously (before it was removed so we could more efficiently set metadata on the various related files).
// This version should take priority, so we remove it from nonForwardableMetadata if it's there to prevent the correct value from
// being overwritten.
nonForwardableMetadata?.Remove(ItemMetadataNames.winMDFileType);
if (reference.IsManagedWinMDFile)
{
referenceItem.SetMetadata(ItemMetadataNames.winMDFileType, "Managed");
}
else
{
referenceItem.SetMetadata(ItemMetadataNames.winMDFileType, "Native");
}

// This may have been set previously (before it was removed so we could more efficiently set metadata on the various related files).
// This version should take priority, so we remove it from nonForwardableMetadata if it's there to prevent the correct value from
// being overwritten.
nonForwardableMetadata?.Remove(ItemMetadataNames.winMDFile);
referenceItem.SetMetadata(ItemMetadataNames.winMDFile, "true");
}

// Set the FusionName late, so we don't copy it to the derived items, but it's still available on referenceItem.
referenceItem.SetMetadata(ItemMetadataNames.fusionName, fusionName);

// nonForwardableMetadata should be null here if relatedFileExtensions, satellites, serializationAssemblyFiles, and scatterFiles were all empty.
if (nonForwardableMetadata != null)
{
foreach (KeyValuePair<string, string> kvp in nonForwardableMetadata)
{
referenceItem.SetMetadata(kvp.Key, kvp.Value);
}
}

return referenceItem;
}

Expand Down Expand Up @@ -3061,15 +3045,28 @@ internal static UInt16 ReadMachineTypeFromPEHeader(string dllPath)
/// <summary>
/// Some metadata should not be forwarded between the parent and child items.
/// </summary>
private static void RemoveNonForwardableMetadata(ITaskItem item)
/// <returns>The metadata that were removed.</returns>
private static Dictionary<string, string> RemoveNonForwardableMetadata(ITaskItem item)
{
item.RemoveMetadata(ItemMetadataNames.winmdImplmentationFile);
item.RemoveMetadata(ItemMetadataNames.imageRuntime);
item.RemoveMetadata(ItemMetadataNames.winMDFile);
Dictionary<string, string> removedMetadata = new Dictionary<string, string>();
RemoveMetadatum(ItemMetadataNames.winmdImplmentationFile, item, removedMetadata);
RemoveMetadatum(ItemMetadataNames.imageRuntime, item, removedMetadata);
RemoveMetadatum(ItemMetadataNames.winMDFile, item, removedMetadata);
if (!Traits.Instance.EscapeHatches.TargetPathForRelatedFiles)
{
item.RemoveMetadata(ItemMetadataNames.targetPath);
RemoveMetadatum(ItemMetadataNames.targetPath, item, removedMetadata);
}
return removedMetadata;
}

private static void RemoveMetadatum(string key, ITaskItem item, Dictionary<string, string> removedMetadata)
{
string meta = item.GetMetadata(key);
if (!String.IsNullOrEmpty(meta))
{
removedMetadata.Add(key, meta);
}
item.RemoveMetadata(key);
}

/// <summary>
Expand Down

0 comments on commit 8fb627e

Please sign in to comment.