Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Optimize RAR's GetReferenceItems method #5929

Merged
merged 23 commits into from Feb 6, 2021
Merged

Optimize RAR's GetReferenceItems method #5929

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
cfe147c
Iterate over dictionary once
Forgind Dec 2, 2020
17015af
Avoid unnecessary initializations
Forgind Dec 2, 2020
fec708e
Set copy local more efficiently
Forgind Dec 2, 2020
ed2bc5c
referenceItem should have no metadata initially
Forgind Dec 2, 2020
2fc517f
Update larger metadata dictionary
Forgind Dec 2, 2020
1dc3bc5
Set fusionName at end
Forgind Dec 2, 2020
77b1038
Slightly simplify referenceItem creation
Forgind Dec 2, 2020
507214d
Fix typos
Forgind Dec 2, 2020
4945ad2
Addition to setting fusionName at end
Forgind Dec 2, 2020
cf314ab
Shift other types of removed metadata later so they only have to be r…
Forgind Dec 2, 2020
7077b0e
Tests should permit case-insensitive metadata checks
Forgind Dec 3, 2020
afe2cf0
Use ternary operators
Forgind Dec 4, 2020
9dccb26
Set items as a group
Forgind Dec 4, 2020
147ef43
PR comments
Forgind Dec 4, 2020
2901929
Revert "Tests should permit case-insensitive metadata checks"
Forgind Dec 4, 2020
fd0a93d
PR comments
Forgind Dec 6, 2020
74cf67c
PR feedback
Forgind Dec 7, 2020
8a45a46
Update src/MSBuildTaskHost/Immutable/ImmutableDictionary.cs
Forgind Dec 7, 2020
2a8893c
Correct setting items
Forgind Dec 7, 2020
04e72ed
Make align with old way
Forgind Jan 26, 2021
2bb3210
Use _metadata directly
Forgind Feb 1, 2021
ff501aa
Merge https://github.com/dotnet/msbuild into optimize-rar
Forgind Feb 2, 2021
2f23808
Access remote metadata indirectly
Forgind Feb 2, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 12 additions & 0 deletions src/Shared/CopyOnWriteDictionary.cs
View file
Open in desktop
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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SetItems and Where will also need to be implemented on the .NET 3.5 version of ImmutableDictionary in C:\src\msbuild\src\MSBuildTaskHost\Immutable\ImmutableDictionary.cs.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! Why isn't Where implemented by LINQ as it is for most IEnumerables?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course! I stand corrected, only SetItems is missing.

}

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
20 changes: 10 additions & 10 deletions src/Tasks.UnitTests/AssemblyDependency/Miscellaneous.cs
View file
Open in desktop
Expand Up @@ -3455,7 +3455,7 @@ public void PrimaryFXAssemblyRefIsNotCopyLocal()

Assert.Single(t.ResolvedFiles);
Assert.Equal(Path.Combine(s_myVersion20Path, "System.Data.dll"), t.ResolvedFiles[0].ItemSpec);
Assert.Equal("false", t.ResolvedFiles[0].GetMetadata("CopyLocal"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert all these please since you went back to the ternary.

Assert.Equal("false", t.ResolvedFiles[0].GetMetadata("CopyLocal"), StringComparer.OrdinalIgnoreCase);
}

/// <summary>
Expand Down Expand Up @@ -3491,7 +3491,7 @@ public void PrivateItemInFrameworksGetsCopyLocalTrue()
t.TargetFrameworkDirectories = new string[] { s_myVersion20Path };
t.SearchPaths = DefaultPaths;
Execute(t);
Assert.Equal(@"true", t.ResolvedFiles[0].GetMetadata("CopyLocal"));
Assert.Equal("true", t.ResolvedFiles[0].GetMetadata("CopyLocal"), StringComparer.OrdinalIgnoreCase);
}

/// <summary>
Expand All @@ -3517,7 +3517,7 @@ public void NoFrameworkDirectoriesStillCopyLocal()
t.TargetFrameworkDirectories = new string[] { };
t.SearchPaths = new string[] { "{RawFileName}" };
Execute(t);
Assert.Equal(@"true", t.ResolvedFiles[0].GetMetadata("CopyLocal"));
Assert.Equal("true", t.ResolvedFiles[0].GetMetadata("CopyLocal"), StringComparer.OrdinalIgnoreCase);
}

/// <summary>
Expand Down Expand Up @@ -4350,7 +4350,7 @@ public void RegressQFE626()
{
if (String.Equals(item.ItemSpec, s_myLibraries_V1_E_EDllPath, StringComparison.OrdinalIgnoreCase))
{
Assert.Equal("false", item.GetMetadata("CopyLocal"));
Assert.Equal("false", item.GetMetadata("CopyLocal"), StringComparer.OrdinalIgnoreCase);
}
}
}
Expand Down Expand Up @@ -4413,12 +4413,12 @@ public void Regress265054()
{
if (String.Equals(item.ItemSpec, s_myLibraries_V1_DDllPath, StringComparison.OrdinalIgnoreCase))
{
Assert.Equal("false", item.GetMetadata("CopyLocal"));
Assert.Equal("false", item.GetMetadata("CopyLocal"), StringComparer.OrdinalIgnoreCase);
}

if (String.Equals(item.ItemSpec, s_myLibraries_V1_E_EDllPath, StringComparison.OrdinalIgnoreCase))
{
Assert.Equal("true", item.GetMetadata("CopyLocal"));
Assert.Equal("true", item.GetMetadata("CopyLocal"), StringComparer.OrdinalIgnoreCase);
}
}
}
Expand Down Expand Up @@ -5350,7 +5350,7 @@ public void Regress435487_FxFileResolvedByHintPathShouldByCopyLocal()
File.Delete(redistFile);
}

Assert.Equal("true", t.ResolvedFiles[0].GetMetadata("CopyLocal")); // "Expected CopyLocal==true."
Assert.Equal("true", t.ResolvedFiles[0].GetMetadata("CopyLocal"), StringComparer.OrdinalIgnoreCase); // "Expected CopyLocal==true."
}

/// <summary>
Expand Down Expand Up @@ -7940,9 +7940,9 @@ public void ForwardRedistRoot()
}

Assert.Equal(3, t.ResolvedFiles.Length); // "Expected three assemblies to be found."
Assert.Equal("true", t.ResolvedFiles[1].GetMetadata("IsRedistRoot"));
Assert.Equal("false", t.ResolvedFiles[0].GetMetadata("IsRedistRoot"));
Assert.Equal("", t.ResolvedFiles[2].GetMetadata("IsRedistRoot"));
Assert.Equal("true", t.ResolvedFiles[1].GetMetadata("IsRedistRoot"), StringComparer.OrdinalIgnoreCase);
Assert.Equal("false", t.ResolvedFiles[0].GetMetadata("IsRedistRoot"), StringComparer.OrdinalIgnoreCase);
Assert.Equal("", t.ResolvedFiles[2].GetMetadata("IsRedistRoot"), StringComparer.OrdinalIgnoreCase);

Assert.Equal("Microsoft-Windows-CLRCoreComp", t.ResolvedFiles[0].GetMetadata("Redist"));
Assert.Equal("Microsoft-Windows-CLRCoreComp", t.ResolvedFiles[1].GetMetadata("Redist"));
Expand Down