Skip to content
This repository has been archived by the owner on Dec 18, 2017. It is now read-only.

#2 : Low hanging fruit performance improvements from profiling session #1188

Merged
merged 1 commit into from
Feb 11, 2015
Merged

#2 : Low hanging fruit performance improvements from profiling session #1188

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
56 changes: 29 additions & 27 deletions src/Microsoft.Framework.Runtime/DependencyManagement/NuGetDependencyResolver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,8 @@ public void Initialize(IEnumerable<LibraryDescription> packages, FrameworkName t
var cacheResolvers = GetCacheResolvers();
var defaultResolver = new DefaultPackagePathResolver(_repository.RepositoryRoot);

Servicing.Breadcrumbs breadcrumbs = new Servicing.Breadcrumbs();
breadcrumbs.CreateRuntimeBreadcrumb();
Servicing.Breadcrumbs breadcrumbs = new Servicing.Breadcrumbs();
breadcrumbs.CreateRuntimeBreadcrumb();

foreach (var dependency in packages)
{
Expand Down Expand Up @@ -177,10 +177,10 @@ public void Initialize(IEnumerable<LibraryDescription> packages, FrameworkName t
packageDescription.ContractPath = contractPath;
}

if (Servicing.Breadcrumbs.IsPackageServiceable(packageDescription.Package))
{
breadcrumbs.CreateBreadcrumb(package.Id, package.Version);
}
if (Servicing.Breadcrumbs.IsPackageServiceable(packageDescription.Package))
{
breadcrumbs.CreateBreadcrumb(package.Id, package.Version);
}

var assemblies = new List<string>();

Expand Down Expand Up @@ -220,32 +220,21 @@ private string ResolvePackagePath(IPackagePathResolver defaultResolver,
IEnumerable<IPackagePathResolver> cacheResolvers,
IPackage package)
{
var defaultHashPath = defaultResolver.GetHashPath(package.Id, package.Version);
string expectedHash = null;
if (File.Exists(defaultHashPath))
{
expectedHash = File.ReadAllText(defaultHashPath);
}
else if (_globalSettings != null)
{
var library = new Library()
{
Name = package.Id,
Version = package.Version
};

_globalSettings.PackageHashes.TryGetValue(library, out expectedHash);
}

if (string.IsNullOrEmpty(expectedHash))
{
return defaultResolver.GetInstallPath(package.Id, package.Version);
}

foreach (var resolver in cacheResolvers)
{
var cacheHashFile = resolver.GetHashPath(package.Id, package.Version);

if (string.IsNullOrEmpty(expectedHash))
{
expectedHash = GetExpectedHash(defaultResolver, package);
}
else
{
break;
}

// REVIEW: More efficient compare?
if (File.Exists(cacheHashFile) &&
File.ReadAllText(cacheHashFile) == expectedHash)
Expand All @@ -257,6 +246,19 @@ private string ResolvePackagePath(IPackagePathResolver defaultResolver,
return defaultResolver.GetInstallPath(package.Id, package.Version);
}

private string GetExpectedHash(IPackagePathResolver defaultResolver, IPackage package)
{
var defaultHashPath = defaultResolver.GetHashPath(package.Id, package.Version);
string expectedHash = null;

if (File.Exists(defaultHashPath))
{
expectedHash = File.ReadAllText(defaultHashPath);
}

return expectedHash;
}

private static IEnumerable<IPackagePathResolver> GetCacheResolvers()
{
var packageCachePathValue = Environment.GetEnvironmentVariable(EnvironmentNames.PackagesCache);
Expand Down Expand Up @@ -467,4 +469,4 @@ private class AssemblyDescription
public string RelativePath { get; set; }
}
}
}
}
45 changes: 34 additions & 11 deletions src/Microsoft.Framework.Runtime/NuGet/SemanticVersion.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

using System;
using System.Globalization;
using System.Text.RegularExpressions;
using NuGet.Resources;

namespace NuGet
Expand All @@ -14,9 +13,6 @@ namespace NuGet
/// </summary>
public sealed class SemanticVersion : IComparable, IComparable<SemanticVersion>, IEquatable<SemanticVersion>
{
private const RegexOptions _flags = RegexOptions.IgnoreCase | RegexOptions.ExplicitCapture;
private static readonly Regex _semanticVersionRegex = new Regex(@"^(?<Version>\d+(\s*\.\s*\d+){0,3})(?<Release>-[a-z][0-9a-z-]*)?$", _flags);
private static readonly Regex _strictSemanticVersionRegex = new Regex(@"^(?<Version>\d+(\.\d+){2})(?<Release>-[a-z][0-9a-z-]*)?$", _flags);
private readonly string _originalString;

public SemanticVersion(string version)
Expand Down Expand Up @@ -149,33 +145,60 @@ public static SemanticVersion Parse(string version)
/// </summary>
public static bool TryParse(string version, out SemanticVersion value)
{
return TryParseInternal(version, _semanticVersionRegex, out value);
return TryParseInternal(version, strict: false, semVer: out value);
}

/// <summary>
/// Parses a version string using strict semantic versioning rules that allows exactly 3 components and an optional special version.
/// </summary>
public static bool TryParseStrict(string version, out SemanticVersion value)
{
return TryParseInternal(version, _strictSemanticVersionRegex, out value);
return TryParseInternal(version, strict: true, semVer: out value);
}

private static bool TryParseInternal(string version, Regex regex, out SemanticVersion semVer)
private static bool TryParseInternal(string version, bool strict, out SemanticVersion semVer)
{
semVer = null;
if (String.IsNullOrEmpty(version))
if (string.IsNullOrEmpty(version))
{
return false;
}

var match = regex.Match(version.Trim());
version = version.Trim();
var versionPart = version;

string specialVersion = string.Empty;
if (version.IndexOf('-') != -1)
{
var parts = version.Split(new char[] { '-' }, 2, StringSplitOptions.RemoveEmptyEntries);
if (parts.Length != 2)
{
return false;
}

versionPart = parts[0];
specialVersion = parts[1];
}

Version versionValue;
if (!match.Success || !Version.TryParse(match.Groups["Version"].Value, out versionValue))
if (!Version.TryParse(versionPart, out versionValue))
{
return false;
}

semVer = new SemanticVersion(NormalizeVersionValue(versionValue), match.Groups["Release"].Value.TrimStart('-'), version.Replace(" ", ""));
if (strict)
{
// Must have major, minor and build only.
if (versionValue.Major == -1 ||
versionValue.Minor == -1 ||
versionValue.Build == -1 ||
versionValue.Revision != -1)
{
return false;
}
}

semVer = new SemanticVersion(NormalizeVersionValue(versionValue), specialVersion, version.Replace(" ", ""));
return true;
}

Expand Down
43 changes: 38 additions & 5 deletions src/Microsoft.Framework.Runtime/NuGet/Utility/PackageIdValidator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,56 @@

using System;
using System.Globalization;
using System.Text.RegularExpressions;
using NuGet.Resources;

namespace NuGet
{
public static class PackageIdValidator
{
internal const int MaxPackageIdLength = 100;
private static readonly Regex _idRegex = new Regex(@"^\w+([_.-]\w+)*$", RegexOptions.IgnoreCase | RegexOptions.ExplicitCapture);

public static bool IsValidPackageId(string packageId)
{
if (packageId == null)
if (string.IsNullOrWhiteSpace(packageId))
{
throw new ArgumentNullException("packageId");
throw new ArgumentException("packageId");
}
return _idRegex.IsMatch(packageId);

// Rules:
// Should start with a character
// Can be followed by '.' or '-'. Cannot have 2 of these special characters consecutively.
// Cannot end with '-' or '.'

var firstChar = packageId[0];
if (!char.IsLetterOrDigit(firstChar) && firstChar != '_')
Copy link
Contributor

Choose a reason for hiding this comment

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

IsLetterOrDigit returns true for _ ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope. Returns false for _.

{
// Should start with a char/digit/_.
return false;
}

var lastChar = packageId[packageId.Length - 1];
if (lastChar == '-' || lastChar == '.')
Copy link
Contributor

Choose a reason for hiding this comment

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

Can the last char be _?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah. w+ includes _.

{
// Should not end with a '-' or '.'.
return false;
}

for (int index = 1; index < packageId.Length - 1; index++)
{
var ch = packageId[index];
if (!char.IsLetterOrDigit(ch) || ch != '-' || ch != '.')
{
return false;
}

if (ch == '-' || ch == '.' && ch == packageId[index - 1])
{
// Cannot have two successive '-' or '.' in the name.
return false;
}
}

return true;
}

public static void ValidatePackageId(string packageId)
Expand Down
22 changes: 18 additions & 4 deletions src/Microsoft.Framework.Runtime/NuGet/Utility/PathValidator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
using System;
using System.Diagnostics.CodeAnalysis;
using System.IO;
using System.Text.RegularExpressions;
using System.Linq;

namespace NuGet
{
Expand Down Expand Up @@ -35,7 +35,10 @@ public static bool IsValidLocalPath(string path)
{
try
{
return Regex.IsMatch(path.Trim(), @"^[A-Za-z]:\\") && Path.IsPathRooted(path) && (path.IndexOfAny(_invalidPathChars) == -1);
return char.IsLetter(path[0]) &&
path.StartsWith(path[0] + ":\\") &&
Path.IsPathRooted(path) &&
(path.IndexOfAny(_invalidPathChars) == -1);
}
catch
{
Expand All @@ -59,7 +62,7 @@ public static bool IsValidUncPath(string path)
try
{
Path.GetFullPath(path);
return Regex.IsMatch(path.Trim(), @"^\\\\");
return path.Trim().StartsWith("\\\\");
}
catch
{
Expand All @@ -78,7 +81,18 @@ public static bool IsValidUrl(string url)
Uri result;

// Make sure url starts with protocol:// because Uri.TryCreate() returns true for local and UNC paths even if badly formed.
return Regex.IsMatch(url, @"^\w+://", RegexOptions.IgnoreCase) && Uri.TryCreate(url, UriKind.Absolute, out result);
var urlParts = url.Split(new string[] { "://" }, 2, StringSplitOptions.None);
if (urlParts == null || urlParts.Length != 2)
{
return false;
}

if (urlParts[0].Any(c => !char.IsLetterOrDigit(c) && c != '_'))
Copy link
Contributor

Choose a reason for hiding this comment

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

This repeats quite a lot !char.IsLetterOrDigit(c) && c != '_', can you extract it to a method?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nah..contexts are bit different. One is url validation, other package name validation etc.

{
return false;
}

return Uri.TryCreate(url, UriKind.Absolute, out result);
}
}
}
15 changes: 9 additions & 6 deletions src/Microsoft.Framework.Runtime/NuGet/Utility/VersionUtility.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,12 +154,17 @@ public static FrameworkName ParseFrameworkName(string frameworkName)
}

// If we find a version then we try to split the framework name into 2 parts
var versionMatch = Regex.Match(frameworkNameAndVersion, @"\d+");
int versionIndex = -1;
var versionMatch = frameworkNameAndVersion.FirstOrDefault(c =>
{
versionIndex++;
return char.IsDigit(c);
});

if (versionMatch.Success)
if (versionMatch != '\0' && versionIndex != -1)
{
identifierPart = frameworkNameAndVersion.Substring(0, versionMatch.Index).Trim();
versionPart = frameworkNameAndVersion.Substring(versionMatch.Index).Trim();
identifierPart = frameworkNameAndVersion.Substring(0, versionIndex).Trim();
versionPart = frameworkNameAndVersion.Substring(versionIndex).Trim();
}
else
{
Expand Down Expand Up @@ -700,8 +705,6 @@ orderby GetProfileCompatibility(internalProjectFramework, g.Key) descending
return hasItems;
}



internal static Version NormalizeVersion(Version verison)
{
return new Version(verison.Major,
Expand Down