Skip to content

Convert IEnumerables to Lists for perf improvements #691

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

Closed
wants to merge 8 commits into from
Closed

Convert IEnumerables to Lists for perf improvements #691

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4a438cd
Update nuget client dependency packages
alerickson Jun 17, 2022
cdf2316
Merge branch 'master' of https://github.com/PowerShell/PowerShellGet
alerickson Jun 22, 2022
3bac8d1
Merge branch 'master' of https://github.com/PowerShell/PowerShellGet
alerickson Jun 24, 2022
e4f9e27
Merge branch 'master' of https://github.com/PowerShell/PowerShellGet
alerickson Jun 29, 2022
082aa21
Change Ienumerables into lists
alerickson Jul 7, 2022
0b0ab60
Merge branch 'master' of https://github.com/powershell/powershellget …
alerickson Jul 19, 2022
e267d4e
Incorporate code review suggestions
alerickson Jul 19, 2022
a19d58c
Change FindByResourceName to return type List instead of IEnumerable
alerickson Jul 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 28 additions & 22 deletions src/code/FindHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public FindHelper(CancellationToken cancellationToken, PSCmdlet cmdletPassedIn)

#region Public methods

public IEnumerable<PSResourceInfo> FindByResourceName(
public List<PSResourceInfo> FindByResourceName(
string[] name,
ResourceType type,
string version,
Expand All @@ -86,12 +86,14 @@ public IEnumerable<PSResourceInfo> FindByResourceName(
_tag = tag;
_credential = credential;
_includeDependencies = includeDependencies;

Dbg.Assert(name.Length != 0, "Name length cannot be 0");

_pkgsLeftToFind = name.ToList();

List<PSRepositoryInfo> repositoriesToSearch;
List<PSResourceInfo> packagesFound = new List<PSResourceInfo>();

if (name.Length == 0)
{
_cmdletPassedIn.WriteVerbose("No name was provided to FindByResourceName.");
}

//determine if repository array of names of repositories input to be searched contains wildcard
if (repository != null)
Expand Down Expand Up @@ -126,7 +128,7 @@ public IEnumerable<PSResourceInfo> FindByResourceName(
"ErrorLoadingRepositoryStoreFile",
ErrorCategory.InvalidArgument,
this));
yield break;
return packagesFound;
}

// loop through repositoriesToSearch and if PSGallery or PoshTestGallery add its Scripts endpoint repo
Expand Down Expand Up @@ -185,9 +187,11 @@ public IEnumerable<PSResourceInfo> FindByResourceName(
repositoryUri: repositoriesToSearch[i].Uri,
repositoryCredentialInfo: repositoriesToSearch[i].CredentialInfo))
{
yield return pkg;
packagesFound.Add(pkg);
}
}

return packagesFound;
}

#endregion
Expand Down Expand Up @@ -351,7 +355,7 @@ private IEnumerable<PSResourceInfo> FindFromPackageSourceSearchAPI(
if (!pkgName.Contains("*"))
{
// case: searching for specific package name i.e "Carbon"
IEnumerable<IPackageSearchMetadata> retrievedPkgs = null;
List<IPackageSearchMetadata> retrievedPkgs = null;
try
{
// GetMetadataAsync() API returns all versions for a specific non-wildcard package name
Expand All @@ -362,7 +366,7 @@ private IEnumerable<PSResourceInfo> FindFromPackageSourceSearchAPI(
includeUnlisted: false,
sourceCacheContext: sourceContext,
log: NullLogger.Instance,
token: _cancellationToken).GetAwaiter().GetResult();
token: _cancellationToken).GetAwaiter().GetResult().ToList();
}
catch (HttpRequestException ex)
{
Expand All @@ -384,7 +388,7 @@ private IEnumerable<PSResourceInfo> FindFromPackageSourceSearchAPI(
yield break;
}

foundPackagesMetadata.AddRange(retrievedPkgs.ToList());
foundPackagesMetadata.AddRange(retrievedPkgs);

// _pkgsLeftToFind.Remove(pkgName);

Expand All @@ -405,7 +409,7 @@ private IEnumerable<PSResourceInfo> FindFromPackageSourceSearchAPI(
yield break;
}
// case: searching for name containing wildcard i.e "Carbon.*"
IEnumerable<IPackageSearchMetadata> wildcardPkgs = null;
List<IPackageSearchMetadata> wildcardPkgs = null;
try
{
// SearchAsync() API returns the latest version only for all packages that match the wild-card name
Expand All @@ -415,17 +419,19 @@ private IEnumerable<PSResourceInfo> FindFromPackageSourceSearchAPI(
skip: 0,
take: SearchAsyncMaxTake,
log: NullLogger.Instance,
cancellationToken: _cancellationToken).GetAwaiter().GetResult();
cancellationToken: _cancellationToken).GetAwaiter().GetResult().ToList();

if (wildcardPkgs.Count() > SearchAsyncMaxReturned)
{
// get the rest of the packages
wildcardPkgs = wildcardPkgs.Concat(pkgSearchResource.SearchAsync(
searchTerm: pkgName,
filters: searchFilter,
skip: SearchAsyncMaxTake,
take: GalleryMax,
log: NullLogger.Instance,
cancellationToken: _cancellationToken).GetAwaiter().GetResult());
wildcardPkgs.AddRange(
pkgSearchResource.SearchAsync(
searchTerm: pkgName,
filters: searchFilter,
skip: SearchAsyncMaxTake,
take: GalleryMax,
log: NullLogger.Instance,
cancellationToken: _cancellationToken).GetAwaiter().GetResult());
}
}
catch (HttpRequestException ex)
Expand All @@ -448,7 +454,7 @@ private IEnumerable<PSResourceInfo> FindFromPackageSourceSearchAPI(
// perhaps validate in Find-PSResource, and use debugassert here?
WildcardPattern nameWildcardPattern = new WildcardPattern(pkgName, WildcardOptions.IgnoreCase);
foundPackagesMetadata.AddRange(wildcardPkgs.Where(
p => nameWildcardPattern.IsMatch(p.Identity.Id)).ToList());
p => nameWildcardPattern.IsMatch(p.Identity.Id)));

if (!_repositoryNameContainsWildcard)
{
Expand Down Expand Up @@ -605,13 +611,13 @@ SourceCacheContext sourceCacheContext
{
foreach(var dep in currentPkg.Dependencies)
{
IEnumerable<IPackageSearchMetadata> depPkgs = packageMetadataResource.GetMetadataAsync(
List<IPackageSearchMetadata> depPkgs = packageMetadataResource.GetMetadataAsync(
packageId: dep.Name,
includePrerelease: _prerelease,
includeUnlisted: false,
sourceCacheContext: sourceCacheContext,
log: NullLogger.Instance,
token: _cancellationToken).GetAwaiter().GetResult();
token: _cancellationToken).GetAwaiter().GetResult().ToList();

if (depPkgs.Count() > 0)
{
Expand Down
34 changes: 19 additions & 15 deletions src/code/InstallHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,17 +192,17 @@ private List<PSResourceInfo> ProcessRepositories(
var isLocalRepo = repo.Uri.AbsoluteUri.StartsWith(Uri.UriSchemeFile + Uri.SchemeDelimiter, StringComparison.OrdinalIgnoreCase);

// Finds parent packages and dependencies
IEnumerable<PSResourceInfo> pkgsFromRepoToInstall = findHelper.FindByResourceName(
List<PSResourceInfo> pkgsFromRepoToInstall = findHelper.FindByResourceName(
name: _pkgNamesToInstall.ToArray(),
type: ResourceType.None,
version: _versionRange != null ? _versionRange.OriginalString : null,
prerelease: _prerelease,
tag: null,
repository: new string[] { repoName },
credential: credential,
includeDependencies: !skipDependencyCheck);
includeDependencies: !skipDependencyCheck).ToList();

if (!pkgsFromRepoToInstall.Any())
if (pkgsFromRepoToInstall.Count == 0)
{
_cmdletPassedIn.WriteVerbose(string.Format("None of the specified resources were found in the '{0}' repository.", repoName));
// Check in the next repository
Expand All @@ -225,7 +225,7 @@ private List<PSResourceInfo> ProcessRepositories(
pkgsFromRepoToInstall = FilterByInstalledPkgs(pkgsFromRepoToInstall);
}

if (!pkgsFromRepoToInstall.Any())
if (pkgsFromRepoToInstall.Count == 0)
{
continue;
}
Expand Down Expand Up @@ -261,7 +261,7 @@ private List<PSResourceInfo> ProcessRepositories(
}

// Check if any of the pkg versions are already installed, if they are we'll remove them from the list of packages to install
private IEnumerable<PSResourceInfo> FilterByInstalledPkgs(IEnumerable<PSResourceInfo> packages)
private List<PSResourceInfo> FilterByInstalledPkgs(List<PSResourceInfo> packages)
{
// Create list of installation paths to search.
List<string> _pathsToSearch = new List<string>();
Expand All @@ -277,21 +277,24 @@ private IEnumerable<PSResourceInfo> FilterByInstalledPkgs(IEnumerable<PSResource
_pathsToSearch.AddRange(Utils.GetSubDirectories(path));
}

var filteredPackages = new Dictionary<string, PSResourceInfo>();
foreach (var pkg in packages)
var filteredPackages = new HashSet<PSResourceInfo>(packages);

List<string> pkgNames = new List<string>();
foreach (PSResourceInfo pkg in packages)
{
filteredPackages.Add(pkg.Name, pkg);
pkgNames.Add(pkg.Name);
}

GetHelper getHelper = new GetHelper(_cmdletPassedIn);
// Get currently installed packages.
// selectPrereleaseOnly is false because even if Prerelease is true we want to include both stable and prerelease, never select prerelease only.
IEnumerable<PSResourceInfo> pkgsAlreadyInstalled = getHelper.GetPackagesFromPath(
name: filteredPackages.Keys.ToArray(),
List<PSResourceInfo> pkgsAlreadyInstalled = getHelper.GetPackagesFromPath(
name: pkgNames.ToArray(),
versionRange: _versionRange,
pathsToSearch: _pathsToSearch,
selectPrereleaseOnly: false);
if (!pkgsAlreadyInstalled.Any())
selectPrereleaseOnly: false).ToList();

if (pkgsAlreadyInstalled.Count == 0)
{
return packages;
}
Expand All @@ -304,15 +307,15 @@ private IEnumerable<PSResourceInfo> FilterByInstalledPkgs(IEnumerable<PSResource
pkg.Name,
pkg.Version));

filteredPackages.Remove(pkg.Name);
filteredPackages.Remove(pkg);
_pkgNamesToInstall.RemoveAll(x => x.Equals(pkg.Name, StringComparison.InvariantCultureIgnoreCase));
}

return filteredPackages.Values.ToArray();
return filteredPackages.ToList();
}

private List<PSResourceInfo> InstallPackage(
IEnumerable<PSResourceInfo> pkgsToInstall, // those found to be required to be installed (includes Dependency packages as well)
List<PSResourceInfo> pkgsToInstall, // those found to be required to be installed (includes Dependency packages as well)
string repoName,
string repoUri,
PSCredentialInfo repoCredentialInfo,
Expand Down Expand Up @@ -698,6 +701,7 @@ private bool DetectClobber(string pkgName, Hashtable parsedMetadataHashtable)
versionRange: VersionRange.All,
pathsToSearch: _pathsToSearch,
selectPrereleaseOnly: false);

// user parsed metadata hash
List<string> listOfCmdlets = new List<string>();
foreach (var cmdletName in parsedMetadataHashtable["CmdletsToExport"] as object[])
Expand Down
6 changes: 3 additions & 3 deletions src/code/UninstallPSResource.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,13 +329,13 @@ private bool CheckIfDependency(string pkgName, out ErrorRecord errorRecord)
// Results is a collection of PSModuleInfo objects that contain a property listing module dependencies, "RequiredModules".
// RequiredModules is collection of PSModuleInfo objects that need to be iterated through to see if any of them are the pkg we're trying to uninstall
// If we anything from the final call gets returned, there is a dependency on this pkg.
IEnumerable<PSObject> pkgsWithRequiredModules = new List<PSObject>();
List<PSObject> pkgsWithRequiredModules = new List<PSObject>();
errorRecord = null;
try
{
pkgsWithRequiredModules = results.Where(
pkgsWithRequiredModules = (results.Where(
pkg => ((ReadOnlyCollection<PSModuleInfo>)pkg.Properties["RequiredModules"].Value).Where(
rm => rm.Name.Equals(pkgName, StringComparison.InvariantCultureIgnoreCase)).Any());
rm => rm.Name.Equals(pkgName, StringComparison.InvariantCultureIgnoreCase)).Any())).ToList();
}
catch (Exception e)
{
Expand Down