From 86213c2df530faf11551ce0852767ddbf71190e8 Mon Sep 17 00:00:00 2001 From: James Luck Date: Thu, 27 Aug 2020 10:03:08 +1000 Subject: [PATCH 01/10] ## Summary Adds caching of git heights. As calculating the height of repositories with large volumes of commits is expensive, caching the git heights can save time in the following circumstances: - Repetitive invocations of the `GetBuildVersion` msbuild task- e.g. during `dotnet pack`'ing a project, the `GetBuildVersion` task is invoked four times - Incremental versioning- in cases where a cached height is available for older commits, this value will be used avoid the cost of recalculating the entire git height (only new commits will need to be traversed) The caching behavior is enabled by default but can be opted-out by setting the new `NerdbankGitVersioningUseHeightCache` msbuild property to `false` ## Testing ### Automated testing - Verified `GitHeightCache` can serialize + deserialize heights correctly - Verified caching has a measurable impact on performance for cases when there are many commits to traverse - All existing tests pass ### Manual testing Consumed locally-packed version of `Nerdbank.GitVersioning` in a C# project and added ~1500 commits, verifying: - Height caching takes effect on second build, dramatically decreasing build time (~10s -> ~1s) - Adding an additional single commit can leverage the cached version for the previous ~1500 commits but adds the latest - Setting the `NerdbankGitVersioningUseHeightCache` property to `false` bypasses the caching behavior. --- .../GitExtensionsTests.cs | 79 ++++++++++++ .../GitHeightCacheTests.cs | 53 ++++++++ .../NerdBank.GitVersioning.Tests.csproj | 1 + src/NerdBank.GitVersioning/GitExtensions.cs | 43 ++++++- src/NerdBank.GitVersioning/GitHeightCache.cs | 114 ++++++++++++++++++ src/NerdBank.GitVersioning/VersionOracle.cs | 20 +-- .../GetBuildVersion.cs | 8 +- .../build/Nerdbank.GitVersioning.targets | 4 +- 8 files changed, 306 insertions(+), 16 deletions(-) create mode 100644 src/NerdBank.GitVersioning.Tests/GitHeightCacheTests.cs create mode 100644 src/NerdBank.GitVersioning/GitHeightCache.cs diff --git a/src/NerdBank.GitVersioning.Tests/GitExtensionsTests.cs b/src/NerdBank.GitVersioning.Tests/GitExtensionsTests.cs index b27d3889..fefbe0af 100644 --- a/src/NerdBank.GitVersioning.Tests/GitExtensionsTests.cs +++ b/src/NerdBank.GitVersioning.Tests/GitExtensionsTests.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.IO; using System.Linq; using System.Text; @@ -410,6 +411,67 @@ public void GetVersionHeight_VeryLongHistory() this.Repo.GetVersionHeight(); } + + [Fact] + public void GetVersionHeight_CachingPerf() + { + const string repoRelativeSubDirectory = "subdir"; + + var semanticVersion1 = SemanticVersion.Parse("1.0"); + this.WriteVersionFile( + new VersionOptions { Version = semanticVersion1 }, + repoRelativeSubDirectory); + + // Add a large volume of commits where the versison file hasn't been bumped- key thing is that when we try to determine the git height, + // we have a lot of commits to walk + const int numCommitsToTraverse = 100; + MeasureRuntime( + () => + { + for (int i = 0; i < numCommitsToTraverse; i++) + this.Repo.Commit($"Test commit #{i}", this.Signer, this.Signer, new CommitOptions {AllowEmptyCommit = true}); + + return -1; + }, + $"Add {numCommitsToTraverse} commits to the git history" + ); + + // First calculation of height will not have the benefit of the cache, will be slow + var (initialHeight, initialTime) = MeasureRuntime(() => this.Repo.Head.GetVersionHeight(repoRelativeSubDirectory), "get the initial (uncached) version height"); + // Second calculation of height should be able to use the cache file generated by the previous calculation + var (cachedHeight, cachedTime) = MeasureRuntime(() => this.Repo.Head.GetVersionHeight(repoRelativeSubDirectory), "get the version height for the unmodified repository (should be cached)"); + + // Want to see at least 20x perf increase + Assert.InRange(cachedTime, TimeSpan.Zero, TimeSpan.FromTicks(initialTime.Ticks / 20)); + Assert.Equal(initialHeight, numCommitsToTraverse + 1); + Assert.Equal(initialHeight, cachedHeight); + + // Adding an additional commit and then getting the height should only involve walking a single commit + this.Repo.Commit($"Final Test commit", this.Signer, this.Signer, new CommitOptions {AllowEmptyCommit = true}); + // Third calculation of height should be able to use the cache file for the first set of commits but walk the last commit to determine the height + (cachedHeight, cachedTime) = MeasureRuntime(() => this.Repo.Head.GetVersionHeight(repoRelativeSubDirectory), "get the version height for the modified repository (should partially use the cache)"); + Assert.Equal(cachedHeight, numCommitsToTraverse + 2); + // We'd expect a less dramatic perf increase this time but should still be significant + Assert.InRange(cachedTime, TimeSpan.Zero, TimeSpan.FromTicks(initialTime.Ticks / 10)); + } + + [Fact] + public void GetVersionHeight_NewCommitsInvalidateCache() + { + const string repoRelativeSubDirectory = "subdir"; + + var semanticVersion1 = SemanticVersion.Parse("1.0"); + this.WriteVersionFile( + new VersionOptions { Version = semanticVersion1 }, + repoRelativeSubDirectory); + + var height1 = this.Repo.Head.GetVersionHeight(repoRelativeSubDirectory); + this.Repo.Commit($"Test commit (should invalidate cache)", this.Signer, this.Signer, new CommitOptions {AllowEmptyCommit = true}); + var height2 = this.Repo.Head.GetVersionHeight(repoRelativeSubDirectory); + + Assert.Equal(1, height1); + Assert.Equal(2, height2); + } [Fact] public void GetCommitsFromVersion_WithPathFilters() @@ -825,4 +887,21 @@ private void VerifyCommitsWithVersion(Commit[] commits) Assert.Equal(commits[i], this.Repo.GetCommitFromVersion(encodedVersion)); } } + + private (T, TimeSpan) MeasureRuntime(Func toTime, string description) + { + var sp = Stopwatch.StartNew(); + try + { + var result = toTime(); + sp.Stop(); + + return (result, sp.Elapsed); + } + finally + { + sp.Stop(); + this.Logger.WriteLine($"Took {sp.Elapsed} to {description}"); + } + } } diff --git a/src/NerdBank.GitVersioning.Tests/GitHeightCacheTests.cs b/src/NerdBank.GitVersioning.Tests/GitHeightCacheTests.cs new file mode 100644 index 00000000..00daf4ac --- /dev/null +++ b/src/NerdBank.GitVersioning.Tests/GitHeightCacheTests.cs @@ -0,0 +1,53 @@ +using System.IO; +using LibGit2Sharp; +using Xunit; +using Version = System.Version; + +namespace Nerdbank.GitVersioning +{ + public class GitHeightCacheTests + { + [Fact] + public void CachedHeightAvailable_NoCacheFile() + { + var cache = new GitHeightCache(Directory.GetCurrentDirectory(), "non-existent-dir"); + Assert.False(cache.CachedHeightAvailable); + } + + [Fact] + public void CachedHeightAvailable_RootCacheFile() + { + File.WriteAllText($"./{GitHeightCache.CacheFileName}", ""); + var cache = new GitHeightCache(Directory.GetCurrentDirectory(), null); + Assert.True(cache.CachedHeightAvailable); + } + + [Fact] + public void CachedHeightAvailable_CacheFile() + { + Directory.CreateDirectory("./testDir"); + File.WriteAllText($"./testDir/{GitHeightCache.CacheFileName}", ""); + var cache = new GitHeightCache(Directory.GetCurrentDirectory(),"testDir/"); + Assert.True(cache.CachedHeightAvailable); + } + + [Fact] + public void GitHeightCache_RoundtripCaching() + { + var cache = new GitHeightCache(Directory.GetCurrentDirectory(), null); + + // test initial set + cache.SetHeight(new ObjectId("8b1f731de6b98aaf536085a62c40dfd3e38817b6"), 2, Version.Parse("1.0.0")); + var cachedHeight = cache.GetHeight(); + Assert.Equal("8b1f731de6b98aaf536085a62c40dfd3e38817b6", cachedHeight.CommitId.Sha); + Assert.Equal(2, cachedHeight.Height); + Assert.Equal("1.0.0", cachedHeight.BaseVersion.ToString()); + + // verify overwriting works correctly + cache.SetHeight(new ObjectId("352459698e082aebef799d77807961d222e75efe"), 3, Version.Parse("1.1.0")); + cachedHeight = cache.GetHeight(); + Assert.Equal("352459698e082aebef799d77807961d222e75efe", cachedHeight.CommitId.Sha); + Assert.Equal("1.1.0", cachedHeight.BaseVersion.ToString()); + } + } +} \ No newline at end of file diff --git a/src/NerdBank.GitVersioning.Tests/NerdBank.GitVersioning.Tests.csproj b/src/NerdBank.GitVersioning.Tests/NerdBank.GitVersioning.Tests.csproj index a0a31aeb..7887a073 100644 --- a/src/NerdBank.GitVersioning.Tests/NerdBank.GitVersioning.Tests.csproj +++ b/src/NerdBank.GitVersioning.Tests/NerdBank.GitVersioning.Tests.csproj @@ -29,6 +29,7 @@ + diff --git a/src/NerdBank.GitVersioning/GitExtensions.cs b/src/NerdBank.GitVersioning/GitExtensions.cs index 66b2d4d1..4104d78f 100644 --- a/src/NerdBank.GitVersioning/GitExtensions.cs +++ b/src/NerdBank.GitVersioning/GitExtensions.cs @@ -40,7 +40,7 @@ public static class GitExtensions /// The repo-relative project directory for which to calculate the version. /// Optional base version to calculate the height. If not specified, the base version will be calculated by scanning the repository. /// The height of the commit. Always a positive integer. - public static int GetVersionHeight(this Commit commit, string repoRelativeProjectDirectory = null, Version baseVersion = null) + public static int GetVersionHeight(this Commit commit, string repoRelativeProjectDirectory = null, Version baseVersion = null, bool useHeightCaching = true) { Requires.NotNull(commit, nameof(commit)); Requires.Argument(repoRelativeProjectDirectory == null || !Path.IsPathRooted(repoRelativeProjectDirectory), nameof(repoRelativeProjectDirectory), "Path should be relative to repo root."); @@ -56,12 +56,45 @@ public static int GetVersionHeight(this Commit commit, string repoRelativeProjec var baseSemVer = baseVersion != null ? SemanticVersion.Parse(baseVersion.ToString()) : versionOptions.Version ?? SemVer0; - + var versionHeightPosition = versionOptions.VersionHeightPosition; + if (versionHeightPosition.HasValue) { - int height = commit.GetHeight(repoRelativeProjectDirectory, c => CommitMatchesVersion(c, baseSemVer, versionHeightPosition.Value, tracker)); - return height; + var cache = new GitHeightCache(commit.GetRepository().Info.WorkingDirectory, repoRelativeProjectDirectory); + int? height = null; + + CachedHeight cachedHeight = null; + if (useHeightCaching && cache.CachedHeightAvailable && (cachedHeight = cache.GetHeight()) != null) + { + // Cached height exactly matches the current commit + if (cachedHeight.CommitId.Equals(commit.Id)) + return cachedHeight.Height; + + if (cachedHeight.BaseVersion == baseSemVer.Version) + { + // In the case that we have a cached height but it's not for the current commit but the same base version, we can still + // try to utilize the cache- stop walking the tree once we reach the cached commit. + var cachedCommitFound = false; + height = commit.GetHeight(repoRelativeProjectDirectory, + c => !(cachedCommitFound |= (c.Id == cachedHeight.CommitId)) && CommitMatchesVersion(c, baseSemVer, versionHeightPosition.Value, tracker)); + + // If we reached the cached commit, include it's cached height + if (cachedCommitFound) + height += cachedHeight.Height; + } + } + + // Cached height either didn't exist or isn't relevant, calculate the full height + if (!height.HasValue) + { + height = commit.GetHeight(repoRelativeProjectDirectory, c => CommitMatchesVersion(c, baseSemVer, versionHeightPosition.Value, tracker)); + } + + if (useHeightCaching) + cache.SetHeight(commit.Id, height.Value, baseSemVer.Version); + + return height.Value; } return 0; @@ -295,7 +328,7 @@ public static Version GetIdAsVersion(this Repository repo, string repoRelativePr if (!versionHeight.HasValue) { var baseVersion = workingCopyVersionOptions?.Version?.Version; - versionHeight = GetVersionHeight(headCommit, repoRelativeProjectDirectory, baseVersion); + versionHeight = GetVersionHeight(headCommit, repoRelativeProjectDirectory, baseVersion, useHeightCaching: false); } Version result = GetIdAsVersionHelper(headCommit, workingCopyVersionOptions, versionHeight.Value); diff --git a/src/NerdBank.GitVersioning/GitHeightCache.cs b/src/NerdBank.GitVersioning/GitHeightCache.cs new file mode 100644 index 00000000..18e89e30 --- /dev/null +++ b/src/NerdBank.GitVersioning/GitHeightCache.cs @@ -0,0 +1,114 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Text.RegularExpressions; +using LibGit2Sharp; +using Newtonsoft.Json; +using Newtonsoft.Json.Converters; +using Version = System.Version; + +namespace Nerdbank.GitVersioning +{ + /// + /// Allows caching the height of a commit for a project, reducing repetitive height calculations. + /// + /// + /// Calculating the height of commits can be time consuming for repositories with 1000s of commits between major/ minor version bumps, + /// so caching the height of a commit can save time. This is especially when packaging projects where height calculation must be done multiple times, + /// see https://github.com/dotnet/Nerdbank.GitVersioning/issues/114#issuecomment-669713622. + /// + public class GitHeightCache + { + private readonly string repoRelativeProjectDirectory; + + private static readonly JsonSerializer JsonSerializer = new JsonSerializer() + { + Converters = + { + new VersionConverter(), + new ObjectIdConverter() + } + }; + + public const string CacheFileName = "version.cache.json"; + + private readonly string heightCacheFilePath; + private readonly Lazy cachedHeightAvailable; + + public GitHeightCache(string repositoryPath, string repoRelativeProjectDirectory) + { + this.repoRelativeProjectDirectory = repoRelativeProjectDirectory; + + if (repositoryPath == null) + this.heightCacheFilePath = null; + else + this.heightCacheFilePath = Path.Combine(repositoryPath, repoRelativeProjectDirectory ?? "", CacheFileName); + + this.cachedHeightAvailable = new Lazy(() => this.heightCacheFilePath != null && File.Exists(this.heightCacheFilePath)); + } + + public bool CachedHeightAvailable => cachedHeightAvailable.Value; + + public CachedHeight GetHeight() + { + try + { + using (var sr = File.OpenText(this.heightCacheFilePath)) + using (var jsonReader = new JsonTextReader(sr)) + { + var cachedHeight = JsonSerializer.Deserialize(jsonReader); + + // Indicates that the project the cache is associated with has moved directories- any cached results may be invalid, so discard + if (cachedHeight.RelativeProjectDir != this.repoRelativeProjectDirectory) + return null; + + return cachedHeight; + } + } + catch (Exception ex) + { + throw new InvalidOperationException($"Unexpected error occurred attempting to read and deserialize '{this.heightCacheFilePath}'. Try deleting this file and rebuilding.", ex); + } + } + + public void SetHeight(ObjectId commitId, int height, Version baseVersion) + { + if (this.heightCacheFilePath == null || commitId == null || commitId == ObjectId.Zero || !Directory.Exists(Path.GetDirectoryName(this.heightCacheFilePath))) + return; + + using (var sw = File.CreateText(this.heightCacheFilePath)) + using (var jsonWriter = new JsonTextWriter(sw)) + { + jsonWriter.WriteComment("Cached commit height, created by Nerdbank.GitVersioning. Do not modify."); + JsonSerializer.Serialize(jsonWriter, new CachedHeight(commitId, height, baseVersion, this.repoRelativeProjectDirectory)); + } + } + + private class ObjectIdConverter : JsonConverter + { + public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer) => writer.WriteValue(((ObjectId) value).Sha); + + public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer) => new ObjectId(reader.Value as string); + + public override bool CanConvert(Type objectType) => objectType == typeof(ObjectId); + } + } + + public class CachedHeight + { + public CachedHeight(ObjectId commitId, int height, Version baseVersion, string relativeProjectDir) + { + this.CommitId = commitId; + this.Height = height; + this.BaseVersion = baseVersion; + this.RelativeProjectDir = relativeProjectDir; + } + + public Version BaseVersion { get; } + public int Height { get; } + public ObjectId CommitId { get; } + public string RelativeProjectDir { get; } + + public override string ToString() => $"({CommitId}, {Height}, {BaseVersion})"; + } +} \ No newline at end of file diff --git a/src/NerdBank.GitVersioning/VersionOracle.cs b/src/NerdBank.GitVersioning/VersionOracle.cs index 26a481f5..653e6eda 100644 --- a/src/NerdBank.GitVersioning/VersionOracle.cs +++ b/src/NerdBank.GitVersioning/VersionOracle.cs @@ -1,4 +1,6 @@ -namespace Nerdbank.GitVersioning +using LibGit2Sharp; + +namespace Nerdbank.GitVersioning { using System; using System.Collections.Generic; @@ -28,7 +30,7 @@ public class VersionOracle /// /// Initializes a new instance of the class. /// - public static VersionOracle Create(string projectDirectory, string gitRepoDirectory = null, ICloudBuild cloudBuild = null, int? overrideBuildNumberOffset = null, string projectPathRelativeToGitRepoRoot = null) + public static VersionOracle Create(string projectDirectory, string gitRepoDirectory = null, ICloudBuild cloudBuild = null, int? overrideBuildNumberOffset = null, string projectPathRelativeToGitRepoRoot = null, bool useHeightCaching = true) { Requires.NotNull(projectDirectory, nameof(projectDirectory)); if (string.IsNullOrEmpty(gitRepoDirectory)) @@ -38,22 +40,22 @@ public static VersionOracle Create(string projectDirectory, string gitRepoDirect using (var git = GitExtensions.OpenGitRepo(gitRepoDirectory)) { - return new VersionOracle(projectDirectory, git, null, cloudBuild, overrideBuildNumberOffset, projectPathRelativeToGitRepoRoot); + return new VersionOracle(projectDirectory, git, null, cloudBuild, overrideBuildNumberOffset, projectPathRelativeToGitRepoRoot, useHeightCaching); } } /// /// Initializes a new instance of the class. /// - public VersionOracle(string projectDirectory, LibGit2Sharp.Repository repo, ICloudBuild cloudBuild, int? overrideBuildNumberOffset = null, string projectPathRelativeToGitRepoRoot = null) - : this(projectDirectory, repo, null, cloudBuild, overrideBuildNumberOffset, projectPathRelativeToGitRepoRoot) + public VersionOracle(string projectDirectory, LibGit2Sharp.Repository repo, ICloudBuild cloudBuild, int? overrideBuildNumberOffset = null, string projectPathRelativeToGitRepoRoot = null, bool useHeightCaching = true) + : this(projectDirectory, repo, null, cloudBuild, overrideBuildNumberOffset, projectPathRelativeToGitRepoRoot, useHeightCaching) { } /// /// Initializes a new instance of the class. /// - public VersionOracle(string projectDirectory, LibGit2Sharp.Repository repo, LibGit2Sharp.Commit head, ICloudBuild cloudBuild, int? overrideVersionHeightOffset = null, string projectPathRelativeToGitRepoRoot = null) + public VersionOracle(string projectDirectory, Repository repo, Commit head, ICloudBuild cloudBuild, int? overrideVersionHeightOffset = null, string projectPathRelativeToGitRepoRoot = null, bool useHeightCaching = true) { var relativeRepoProjectDirectory = projectPathRelativeToGitRepoRoot ?? repo?.GetRepoRelativePath(projectDirectory); @@ -80,7 +82,7 @@ public VersionOracle(string projectDirectory, LibGit2Sharp.Repository repo, LibG this.GitCommitId = commit?.Id.Sha ?? cloudBuild?.GitCommitId ?? null; this.GitCommitDate = commit?.Author.When; - this.VersionHeight = CalculateVersionHeight(relativeRepoProjectDirectory, commit, committedVersion, workingVersion); + this.VersionHeight = CalculateVersionHeight(relativeRepoProjectDirectory, commit, committedVersion, workingVersion, useHeightCaching); this.BuildingRef = cloudBuild?.BuildingTag ?? cloudBuild?.BuildingBranch ?? repo?.Head.CanonicalName; // Override the typedVersion with the special build number and revision components, when available. @@ -471,7 +473,7 @@ private static Version GetAssemblyVersion(Version version, VersionOptions versio /// The specified string, with macros substituted for actual values. private string ReplaceMacros(string prereleaseOrBuildMetadata) => prereleaseOrBuildMetadata?.Replace(VersionOptions.VersionHeightPlaceholder, this.VersionHeightWithOffset.ToString(CultureInfo.InvariantCulture)); - private static int CalculateVersionHeight(string relativeRepoProjectDirectory, LibGit2Sharp.Commit headCommit, VersionOptions committedVersion, VersionOptions workingVersion) + private static int CalculateVersionHeight(string relativeRepoProjectDirectory, LibGit2Sharp.Commit headCommit, VersionOptions committedVersion, VersionOptions workingVersion, bool useHeightCaching) { var headCommitVersion = committedVersion?.Version ?? SemVer0; @@ -487,7 +489,7 @@ private static int CalculateVersionHeight(string relativeRepoProjectDirectory, L } } - return headCommit?.GetVersionHeight(relativeRepoProjectDirectory) ?? 0; + return headCommit?.GetVersionHeight(relativeRepoProjectDirectory, useHeightCaching: useHeightCaching) ?? 0; } private static Version GetIdAsVersion(LibGit2Sharp.Commit headCommit, VersionOptions committedVersion, VersionOptions workingVersion, int versionHeight) diff --git a/src/Nerdbank.GitVersioning.Tasks/GetBuildVersion.cs b/src/Nerdbank.GitVersioning.Tasks/GetBuildVersion.cs index 72fad909..a47f3d32 100644 --- a/src/Nerdbank.GitVersioning.Tasks/GetBuildVersion.cs +++ b/src/Nerdbank.GitVersioning.Tasks/GetBuildVersion.cs @@ -55,6 +55,12 @@ public GetBuildVersion() /// Gets or sets the optional override build number offset. /// public int OverrideBuildNumberOffset { get; set; } = int.MaxValue; + + /// + /// Gets or set if the heights of commits should be cached. Caching commit heights can bring + /// significant performance improvements. + /// + public bool UseHeightCaching { get; set; } = true; /// /// Gets or sets the path to the folder that contains the NB.GV .targets file. @@ -202,7 +208,7 @@ protected override bool ExecuteInner() var cloudBuild = CloudBuild.Active; var overrideBuildNumberOffset = (this.OverrideBuildNumberOffset == int.MaxValue) ? (int?)null : this.OverrideBuildNumberOffset; - var oracle = VersionOracle.Create(Directory.GetCurrentDirectory(), this.GitRepoRoot, cloudBuild, overrideBuildNumberOffset, this.ProjectPathRelativeToGitRepoRoot); + var oracle = VersionOracle.Create(Directory.GetCurrentDirectory(), this.GitRepoRoot, cloudBuild, overrideBuildNumberOffset, this.ProjectPathRelativeToGitRepoRoot, this.UseHeightCaching); if (!string.IsNullOrEmpty(this.DefaultPublicRelease)) { oracle.PublicRelease = string.Equals(this.DefaultPublicRelease, "true", StringComparison.OrdinalIgnoreCase); diff --git a/src/Nerdbank.GitVersioning.Tasks/build/Nerdbank.GitVersioning.targets b/src/Nerdbank.GitVersioning.Tasks/build/Nerdbank.GitVersioning.targets index 112d707a..83591248 100644 --- a/src/Nerdbank.GitVersioning.Tasks/build/Nerdbank.GitVersioning.targets +++ b/src/Nerdbank.GitVersioning.Tasks/build/Nerdbank.GitVersioning.targets @@ -27,6 +27,7 @@ <_NBGV_PlatformSuffix Condition=" '$(_NBGV_PlatformSuffix)' == '' and '$(MSBuildRuntimeType)' == 'Core' ">MSBuildCore/ <_NBGV_PlatformSuffix Condition=" '$(_NBGV_PlatformSuffix)' == '' ">MSBuildFull/ $(MSBuildThisFileDirectory)$(_NBGV_PlatformSuffix) + true @@ -75,7 +76,8 @@ GitRepoRoot="$(GitRepoRoot)" ProjectPathRelativeToGitRepoRoot="$(ProjectPathRelativeToGitRepoRoot)" OverrideBuildNumberOffset="$(OverrideBuildNumberOffset)" - TargetsPath="$(MSBuildThisFileDirectory)"> + TargetsPath="$(MSBuildThisFileDirectory)" + UseHeightCaching="$(NerdbankGitVersioningUseHeightCache)"> From e82e23ae6938301fdf4d70da6d20f205717b439a Mon Sep 17 00:00:00 2001 From: James Luck Date: Thu, 27 Aug 2020 10:27:14 +1000 Subject: [PATCH 02/10] Fixing required documentation errors --- src/NerdBank.GitVersioning/GitExtensions.cs | 10 +++-- src/NerdBank.GitVersioning/GitHeightCache.cs | 43 +++++++++++++++++++- 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/src/NerdBank.GitVersioning/GitExtensions.cs b/src/NerdBank.GitVersioning/GitExtensions.cs index 4104d78f..5242b2bf 100644 --- a/src/NerdBank.GitVersioning/GitExtensions.cs +++ b/src/NerdBank.GitVersioning/GitExtensions.cs @@ -39,6 +39,10 @@ public static class GitExtensions /// The commit to measure the height of. /// The repo-relative project directory for which to calculate the version. /// Optional base version to calculate the height. If not specified, the base version will be calculated by scanning the repository. + /// + /// If true, the version height will be cached for subsequent invocations. If a previously cached version height exists and it is valid, it will be used. + /// Defaults to true. + /// /// The height of the commit. Always a positive integer. public static int GetVersionHeight(this Commit commit, string repoRelativeProjectDirectory = null, Version baseVersion = null, bool useHeightCaching = true) { @@ -270,7 +274,7 @@ private static IRepository GetRepository(this IBelongToARepository repositoryMem /// The commit whose ID and position in history is to be encoded. /// The repo-relative project directory for which to calculate the version. /// - /// The version height, previously calculated by a call to + /// The version height, previously calculated by a call to /// with the same value for . /// /// @@ -304,7 +308,7 @@ public static Version GetIdAsVersion(this Commit commit, string repoRelativeProj /// The repo whose ID and position in history is to be encoded. /// The repo-relative project directory for which to calculate the version. /// - /// The version height, previously calculated by a call to + /// The version height, previously calculated by a call to /// with the same value for . /// /// @@ -913,7 +917,7 @@ private static void AddReachableCommitsFrom(Commit startingCommit, HashSet /// The commit whose ID and position in history is to be encoded. /// The version options applicable at this point (either from commit or working copy). - /// The version height, previously calculated by a call to . + /// The version height, previously calculated by a call to . /// /// A version whose and /// components are calculated based on the commit. diff --git a/src/NerdBank.GitVersioning/GitHeightCache.cs b/src/NerdBank.GitVersioning/GitHeightCache.cs index 18e89e30..b5c4b683 100644 --- a/src/NerdBank.GitVersioning/GitHeightCache.cs +++ b/src/NerdBank.GitVersioning/GitHeightCache.cs @@ -30,11 +30,19 @@ public class GitHeightCache } }; + /// + /// The name used for the cache file. + /// public const string CacheFileName = "version.cache.json"; private readonly string heightCacheFilePath; private readonly Lazy cachedHeightAvailable; + /// + /// Creates a new height cache. + /// + /// The root path of the repository. + /// The relative path of the project within the repository. public GitHeightCache(string repositoryPath, string repoRelativeProjectDirectory) { this.repoRelativeProjectDirectory = repoRelativeProjectDirectory; @@ -47,8 +55,16 @@ public GitHeightCache(string repositoryPath, string repoRelativeProjectDirectory this.cachedHeightAvailable = new Lazy(() => this.heightCacheFilePath != null && File.Exists(this.heightCacheFilePath)); } + /// + /// Determines if a cached version is available. + /// public bool CachedHeightAvailable => cachedHeightAvailable.Value; + /// + /// Fetches the . May return null if the cache is not valid. + /// + /// + /// public CachedHeight GetHeight() { try @@ -71,6 +87,12 @@ public CachedHeight GetHeight() } } + /// + /// Caches the height of a commit, overwriting any previously cached values. + /// + /// + /// + /// public void SetHeight(ObjectId commitId, int height, Version baseVersion) { if (this.heightCacheFilePath == null || commitId == null || commitId == ObjectId.Zero || !Directory.Exists(Path.GetDirectoryName(this.heightCacheFilePath))) @@ -94,9 +116,12 @@ private class ObjectIdConverter : JsonConverter } } + /// + /// The cached git height of a project. + /// public class CachedHeight { - public CachedHeight(ObjectId commitId, int height, Version baseVersion, string relativeProjectDir) + internal CachedHeight(ObjectId commitId, int height, Version baseVersion, string relativeProjectDir) { this.CommitId = commitId; this.Height = height; @@ -104,11 +129,27 @@ public CachedHeight(ObjectId commitId, int height, Version baseVersion, string r this.RelativeProjectDir = relativeProjectDir; } + /// + /// The base version this cached height was calculated for. + /// public Version BaseVersion { get; } + + /// + /// The cached height. + /// public int Height { get; } + + /// + /// The commit id for the cached height. + /// public ObjectId CommitId { get; } + + /// + /// The relative path of the project this cached height belong to. + /// public string RelativeProjectDir { get; } + /// public override string ToString() => $"({CommitId}, {Height}, {BaseVersion})"; } } \ No newline at end of file From 925130829eeb8512b255fcd999660cc6011cba45 Mon Sep 17 00:00:00 2001 From: James Luck Date: Thu, 27 Aug 2020 10:37:54 +1000 Subject: [PATCH 03/10] Constructor must be public --- src/NerdBank.GitVersioning/GitHeightCache.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/NerdBank.GitVersioning/GitHeightCache.cs b/src/NerdBank.GitVersioning/GitHeightCache.cs index b5c4b683..310a1c75 100644 --- a/src/NerdBank.GitVersioning/GitHeightCache.cs +++ b/src/NerdBank.GitVersioning/GitHeightCache.cs @@ -121,7 +121,14 @@ private class ObjectIdConverter : JsonConverter /// public class CachedHeight { - internal CachedHeight(ObjectId commitId, int height, Version baseVersion, string relativeProjectDir) + /// + /// + /// + /// + /// + /// + /// + public CachedHeight(ObjectId commitId, int height, Version baseVersion, string relativeProjectDir) { this.CommitId = commitId; this.Height = height; From fad06f0df15240d154b7a8262c59265f1d6e85e8 Mon Sep 17 00:00:00 2001 From: James Luck Date: Thu, 27 Aug 2020 10:46:13 +1000 Subject: [PATCH 04/10] Removing test SDK to see if this is cause for broken build --- .../NerdBank.GitVersioning.Tests.csproj | 1 - 1 file changed, 1 deletion(-) diff --git a/src/NerdBank.GitVersioning.Tests/NerdBank.GitVersioning.Tests.csproj b/src/NerdBank.GitVersioning.Tests/NerdBank.GitVersioning.Tests.csproj index 7887a073..a0a31aeb 100644 --- a/src/NerdBank.GitVersioning.Tests/NerdBank.GitVersioning.Tests.csproj +++ b/src/NerdBank.GitVersioning.Tests/NerdBank.GitVersioning.Tests.csproj @@ -29,7 +29,6 @@ - From 3fe9ec6d608fdac75e94704c496f64d8421918e0 Mon Sep 17 00:00:00 2001 From: James Luck Date: Thu, 27 Aug 2020 12:21:21 +1000 Subject: [PATCH 05/10] Simplifying logic around discarding cache height when base version doesn't match --- .../GitHeightCacheTests.cs | 16 +++++----- src/NerdBank.GitVersioning/GitExtensions.cs | 29 +++++++++---------- src/NerdBank.GitVersioning/GitHeightCache.cs | 16 ++++++---- 3 files changed, 32 insertions(+), 29 deletions(-) diff --git a/src/NerdBank.GitVersioning.Tests/GitHeightCacheTests.cs b/src/NerdBank.GitVersioning.Tests/GitHeightCacheTests.cs index 00daf4ac..4c854dac 100644 --- a/src/NerdBank.GitVersioning.Tests/GitHeightCacheTests.cs +++ b/src/NerdBank.GitVersioning.Tests/GitHeightCacheTests.cs @@ -10,7 +10,7 @@ public class GitHeightCacheTests [Fact] public void CachedHeightAvailable_NoCacheFile() { - var cache = new GitHeightCache(Directory.GetCurrentDirectory(), "non-existent-dir"); + var cache = new GitHeightCache(Directory.GetCurrentDirectory(), "non-existent-dir", new Version(1, 0)); Assert.False(cache.CachedHeightAvailable); } @@ -18,7 +18,7 @@ public void CachedHeightAvailable_NoCacheFile() public void CachedHeightAvailable_RootCacheFile() { File.WriteAllText($"./{GitHeightCache.CacheFileName}", ""); - var cache = new GitHeightCache(Directory.GetCurrentDirectory(), null); + var cache = new GitHeightCache(Directory.GetCurrentDirectory(), null, new Version(1, 0)); Assert.True(cache.CachedHeightAvailable); } @@ -27,27 +27,27 @@ public void CachedHeightAvailable_CacheFile() { Directory.CreateDirectory("./testDir"); File.WriteAllText($"./testDir/{GitHeightCache.CacheFileName}", ""); - var cache = new GitHeightCache(Directory.GetCurrentDirectory(),"testDir/"); + var cache = new GitHeightCache(Directory.GetCurrentDirectory(),"testDir/", new Version(1, 0)); Assert.True(cache.CachedHeightAvailable); } [Fact] public void GitHeightCache_RoundtripCaching() { - var cache = new GitHeightCache(Directory.GetCurrentDirectory(), null); + var cache = new GitHeightCache(Directory.GetCurrentDirectory(), null, new Version(1, 0)); // test initial set - cache.SetHeight(new ObjectId("8b1f731de6b98aaf536085a62c40dfd3e38817b6"), 2, Version.Parse("1.0.0")); + cache.SetHeight(new ObjectId("8b1f731de6b98aaf536085a62c40dfd3e38817b6"), 2); var cachedHeight = cache.GetHeight(); Assert.Equal("8b1f731de6b98aaf536085a62c40dfd3e38817b6", cachedHeight.CommitId.Sha); Assert.Equal(2, cachedHeight.Height); - Assert.Equal("1.0.0", cachedHeight.BaseVersion.ToString()); + Assert.Equal("1.0", cachedHeight.BaseVersion.ToString()); // verify overwriting works correctly - cache.SetHeight(new ObjectId("352459698e082aebef799d77807961d222e75efe"), 3, Version.Parse("1.1.0")); + cache.SetHeight(new ObjectId("352459698e082aebef799d77807961d222e75efe"), 3); cachedHeight = cache.GetHeight(); Assert.Equal("352459698e082aebef799d77807961d222e75efe", cachedHeight.CommitId.Sha); - Assert.Equal("1.1.0", cachedHeight.BaseVersion.ToString()); + Assert.Equal("1.0", cachedHeight.BaseVersion.ToString()); } } } \ No newline at end of file diff --git a/src/NerdBank.GitVersioning/GitExtensions.cs b/src/NerdBank.GitVersioning/GitExtensions.cs index 5242b2bf..8217c4e9 100644 --- a/src/NerdBank.GitVersioning/GitExtensions.cs +++ b/src/NerdBank.GitVersioning/GitExtensions.cs @@ -65,7 +65,7 @@ public static int GetVersionHeight(this Commit commit, string repoRelativeProjec if (versionHeightPosition.HasValue) { - var cache = new GitHeightCache(commit.GetRepository().Info.WorkingDirectory, repoRelativeProjectDirectory); + var cache = new GitHeightCache(commit.GetRepository().Info.WorkingDirectory, repoRelativeProjectDirectory, baseSemVer.Version); int? height = null; CachedHeight cachedHeight = null; @@ -74,19 +74,16 @@ public static int GetVersionHeight(this Commit commit, string repoRelativeProjec // Cached height exactly matches the current commit if (cachedHeight.CommitId.Equals(commit.Id)) return cachedHeight.Height; - - if (cachedHeight.BaseVersion == baseSemVer.Version) - { - // In the case that we have a cached height but it's not for the current commit but the same base version, we can still - // try to utilize the cache- stop walking the tree once we reach the cached commit. - var cachedCommitFound = false; - height = commit.GetHeight(repoRelativeProjectDirectory, - c => !(cachedCommitFound |= (c.Id == cachedHeight.CommitId)) && CommitMatchesVersion(c, baseSemVer, versionHeightPosition.Value, tracker)); - - // If we reached the cached commit, include it's cached height - if (cachedCommitFound) - height += cachedHeight.Height; - } + + // In the case that we have a cached height but it's not for the current commit but the same base version, we can still + // try to utilize the cache- stop walking the tree once we reach the cached commit. + var cachedCommitFound = false; + height = commit.GetHeight(repoRelativeProjectDirectory, + c => !(cachedCommitFound |= (c.Id == cachedHeight.CommitId)) && CommitMatchesVersion(c, baseSemVer, versionHeightPosition.Value, tracker)); + + // If we reached the cached commit, include it's cached height + if (cachedCommitFound) + height += cachedHeight.Height; } // Cached height either didn't exist or isn't relevant, calculate the full height @@ -96,7 +93,7 @@ public static int GetVersionHeight(this Commit commit, string repoRelativeProjec } if (useHeightCaching) - cache.SetHeight(commit.Id, height.Value, baseSemVer.Version); + cache.SetHeight(commit.Id, height.Value); return height.Value; } @@ -332,7 +329,7 @@ public static Version GetIdAsVersion(this Repository repo, string repoRelativePr if (!versionHeight.HasValue) { var baseVersion = workingCopyVersionOptions?.Version?.Version; - versionHeight = GetVersionHeight(headCommit, repoRelativeProjectDirectory, baseVersion, useHeightCaching: false); + versionHeight = GetVersionHeight(headCommit, repoRelativeProjectDirectory, baseVersion); } Version result = GetIdAsVersionHelper(headCommit, workingCopyVersionOptions, versionHeight.Value); diff --git a/src/NerdBank.GitVersioning/GitHeightCache.cs b/src/NerdBank.GitVersioning/GitHeightCache.cs index 310a1c75..c03c1673 100644 --- a/src/NerdBank.GitVersioning/GitHeightCache.cs +++ b/src/NerdBank.GitVersioning/GitHeightCache.cs @@ -20,6 +20,7 @@ namespace Nerdbank.GitVersioning public class GitHeightCache { private readonly string repoRelativeProjectDirectory; + private readonly Version baseVersion; private static readonly JsonSerializer JsonSerializer = new JsonSerializer() { @@ -43,10 +44,12 @@ public class GitHeightCache /// /// The root path of the repository. /// The relative path of the project within the repository. - public GitHeightCache(string repositoryPath, string repoRelativeProjectDirectory) + /// + public GitHeightCache(string repositoryPath, string repoRelativeProjectDirectory, Version baseVersion) { this.repoRelativeProjectDirectory = repoRelativeProjectDirectory; - + this.baseVersion = baseVersion; + if (repositoryPath == null) this.heightCacheFilePath = null; else @@ -73,6 +76,10 @@ public CachedHeight GetHeight() using (var jsonReader = new JsonTextReader(sr)) { var cachedHeight = JsonSerializer.Deserialize(jsonReader); + + // Indicates any cached height is irrelevant- every time the base version is bumped, we need to walk an entirely different set of commits + if (cachedHeight.BaseVersion != this.baseVersion) + return null; // Indicates that the project the cache is associated with has moved directories- any cached results may be invalid, so discard if (cachedHeight.RelativeProjectDir != this.repoRelativeProjectDirectory) @@ -92,8 +99,7 @@ public CachedHeight GetHeight() /// /// /// - /// - public void SetHeight(ObjectId commitId, int height, Version baseVersion) + public void SetHeight(ObjectId commitId, int height) { if (this.heightCacheFilePath == null || commitId == null || commitId == ObjectId.Zero || !Directory.Exists(Path.GetDirectoryName(this.heightCacheFilePath))) return; @@ -102,7 +108,7 @@ public void SetHeight(ObjectId commitId, int height, Version baseVersion) using (var jsonWriter = new JsonTextWriter(sw)) { jsonWriter.WriteComment("Cached commit height, created by Nerdbank.GitVersioning. Do not modify."); - JsonSerializer.Serialize(jsonWriter, new CachedHeight(commitId, height, baseVersion, this.repoRelativeProjectDirectory)); + JsonSerializer.Serialize(jsonWriter, new CachedHeight(commitId, height, this.baseVersion, this.repoRelativeProjectDirectory)); } } From 234625ea4a86be5cd61c3d92cf84f6bd3d3dc03f Mon Sep 17 00:00:00 2001 From: James Luck Date: Thu, 27 Aug 2020 14:57:27 +1000 Subject: [PATCH 06/10] Fixing doco issue --- src/NerdBank.GitVersioning/GitHeightCache.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/NerdBank.GitVersioning/GitHeightCache.cs b/src/NerdBank.GitVersioning/GitHeightCache.cs index c03c1673..32544f91 100644 --- a/src/NerdBank.GitVersioning/GitHeightCache.cs +++ b/src/NerdBank.GitVersioning/GitHeightCache.cs @@ -44,7 +44,7 @@ public class GitHeightCache /// /// The root path of the repository. /// The relative path of the project within the repository. - /// + /// public GitHeightCache(string repositoryPath, string repoRelativeProjectDirectory, Version baseVersion) { this.repoRelativeProjectDirectory = repoRelativeProjectDirectory; From 85eef33a95d51ac787385726054f060a5e62a72e Mon Sep 17 00:00:00 2001 From: James Luck Date: Thu, 27 Aug 2020 15:07:12 +1000 Subject: [PATCH 07/10] Testing adding a reference to compiler.unsafe to see if it will help the build along --- .../NerdBank.GitVersioning.Tests.csproj | 1 + 1 file changed, 1 insertion(+) diff --git a/src/NerdBank.GitVersioning.Tests/NerdBank.GitVersioning.Tests.csproj b/src/NerdBank.GitVersioning.Tests/NerdBank.GitVersioning.Tests.csproj index a0a31aeb..41579b8f 100644 --- a/src/NerdBank.GitVersioning.Tests/NerdBank.GitVersioning.Tests.csproj +++ b/src/NerdBank.GitVersioning.Tests/NerdBank.GitVersioning.Tests.csproj @@ -34,6 +34,7 @@ + From 7202d1b2f860386886bbfd09887772bbc457f6c2 Mon Sep 17 00:00:00 2001 From: James Luck Date: Thu, 27 Aug 2020 15:13:03 +1000 Subject: [PATCH 08/10] More messing around with package versions- this time System.Numerics --- .../NerdBank.GitVersioning.Tests.csproj | 1 + 1 file changed, 1 insertion(+) diff --git a/src/NerdBank.GitVersioning.Tests/NerdBank.GitVersioning.Tests.csproj b/src/NerdBank.GitVersioning.Tests/NerdBank.GitVersioning.Tests.csproj index 41579b8f..d6a3479c 100644 --- a/src/NerdBank.GitVersioning.Tests/NerdBank.GitVersioning.Tests.csproj +++ b/src/NerdBank.GitVersioning.Tests/NerdBank.GitVersioning.Tests.csproj @@ -34,6 +34,7 @@ + From 670f6e7bc122a4517f50a9782bc5d7483df47df6 Mon Sep 17 00:00:00 2001 From: James Luck Date: Fri, 28 Aug 2020 11:42:14 +1000 Subject: [PATCH 09/10] Fixing a bug that meant the incorrect height would be returned for commits with multiple parents (any cached height would be used and could prevent the relevant commit graph being walked) --- .../GitExtensionsTests.cs | 34 +++++++++++++++++- src/NerdBank.GitVersioning/GitExtensions.cs | 36 ++++++++----------- 2 files changed, 48 insertions(+), 22 deletions(-) diff --git a/src/NerdBank.GitVersioning.Tests/GitExtensionsTests.cs b/src/NerdBank.GitVersioning.Tests/GitExtensionsTests.cs index fefbe0af..bacae7e5 100644 --- a/src/NerdBank.GitVersioning.Tests/GitExtensionsTests.cs +++ b/src/NerdBank.GitVersioning.Tests/GitExtensionsTests.cs @@ -424,7 +424,7 @@ public void GetVersionHeight_CachingPerf() // Add a large volume of commits where the versison file hasn't been bumped- key thing is that when we try to determine the git height, // we have a lot of commits to walk - const int numCommitsToTraverse = 100; + const int numCommitsToTraverse = 300; MeasureRuntime( () => { @@ -455,6 +455,38 @@ public void GetVersionHeight_CachingPerf() Assert.InRange(cachedTime, TimeSpan.Zero, TimeSpan.FromTicks(initialTime.Ticks / 10)); } + [Fact] + public void GetVersionHeight_CachingMultipleParents() + { + /* + * Layout of branches + commits: + * master: version -> second --------------> | + * | | + * another: | -> branch commit #n x 5 | -> merge commit + */ + this.WriteVersionFile(); + var anotherBranch = this.Repo.CreateBranch("another"); + var secondCommit = this.Repo.Commit("Second", this.Signer, this.Signer, new CommitOptions { AllowEmptyCommit = true }); + + // get height of the second commit- will cache the height for this commit + var height = secondCommit.GetVersionHeight(useHeightCaching: true); + Assert.Equal(2, height); + + // add many commits to the another branch + merge with master + Commands.Checkout(this.Repo, anotherBranch); + Commit[] branchCommits = new Commit[5]; + for (int i = 1; i <= branchCommits.Length; i++) + { + branchCommits[i - 1] = this.Repo.Commit($"branch commit #{i}", this.Signer, this.Signer, new CommitOptions { AllowEmptyCommit = true }); + } + + this.Repo.Merge(secondCommit, new Signature("t", "t@t.com", DateTimeOffset.Now), new MergeOptions { FastForwardStrategy = FastForwardStrategy.NoFastForward }); + + // The height should be the height of the 'another' branch as it has the greatest height. + // The cached height for the master branch should be ignored. + Assert.Equal(7, this.Repo.Head.GetVersionHeight()); + } + [Fact] public void GetVersionHeight_NewCommitsInvalidateCache() { diff --git a/src/NerdBank.GitVersioning/GitExtensions.cs b/src/NerdBank.GitVersioning/GitExtensions.cs index 8217c4e9..487d49bd 100644 --- a/src/NerdBank.GitVersioning/GitExtensions.cs +++ b/src/NerdBank.GitVersioning/GitExtensions.cs @@ -66,36 +66,28 @@ public static int GetVersionHeight(this Commit commit, string repoRelativeProjec if (versionHeightPosition.HasValue) { var cache = new GitHeightCache(commit.GetRepository().Info.WorkingDirectory, repoRelativeProjectDirectory, baseSemVer.Version); - int? height = null; - CachedHeight cachedHeight = null; if (useHeightCaching && cache.CachedHeightAvailable && (cachedHeight = cache.GetHeight()) != null) { - // Cached height exactly matches the current commit + if (cachedHeight.CommitId.Equals(commit.Id)) + // Cached height exactly matches the current commit return cachedHeight.Height; - - // In the case that we have a cached height but it's not for the current commit but the same base version, we can still - // try to utilize the cache- stop walking the tree once we reach the cached commit. - var cachedCommitFound = false; - height = commit.GetHeight(repoRelativeProjectDirectory, - c => !(cachedCommitFound |= (c.Id == cachedHeight.CommitId)) && CommitMatchesVersion(c, baseSemVer, versionHeightPosition.Value, tracker)); - - // If we reached the cached commit, include it's cached height - if (cachedCommitFound) - height += cachedHeight.Height; - } - - // Cached height either didn't exist or isn't relevant, calculate the full height - if (!height.HasValue) - { - height = commit.GetHeight(repoRelativeProjectDirectory, c => CommitMatchesVersion(c, baseSemVer, versionHeightPosition.Value, tracker)); + else + { + // Cached height doesn't match the current commit. However, we can store the cached height in the walker to avoid walking the full height of the commit graph. + var cachedCommit = commit.GetRepository().Lookup(cachedHeight.CommitId) as Commit; + if (cachedCommit != null) + tracker.RecordHeight(cachedCommit, cachedHeight.Height); + } } + var height = GetCommitHeight(commit, tracker, c => CommitMatchesVersion(c, baseSemVer, versionHeightPosition.Value, tracker)); + if (useHeightCaching) - cache.SetHeight(commit.Id, height.Value); + cache.SetHeight(commit.Id, height); - return height.Value; + return height; } return 0; @@ -763,6 +755,8 @@ private static int GetCommitHeight(Commit startingCommit, GitWalkTracker tracker var commitsToEvaluate = new Stack(); bool TryCalculateHeight(Commit commit) { + // if is cached, then bail? + // Get max height among all parents, or schedule all missing parents for their own evaluation and return false. int maxHeightAmongParents = 0; bool parentMissing = false; From 2b58c634dd6e4d950674501e0d57095380b8a885 Mon Sep 17 00:00:00 2001 From: James Luck Date: Wed, 2 Sep 2020 18:00:58 +1000 Subject: [PATCH 10/10] Adding support for the single version file scenario- git heights are now cached on disk next to the `version.json`/ `version.txt` file in order that performance improvements can be seen by multiple projects. --- .../GitExtensionsTests.cs | 30 +++++++++++++++++-- src/NerdBank.GitVersioning/GitExtensions.cs | 4 +-- src/NerdBank.GitVersioning/GitHeightCache.cs | 24 +++++++-------- src/NerdBank.GitVersioning/VersionFile.cs | 17 ++++++----- src/NerdBank.GitVersioning/VersionOptions.cs | 7 +++++ 5 files changed, 58 insertions(+), 24 deletions(-) diff --git a/src/NerdBank.GitVersioning.Tests/GitExtensionsTests.cs b/src/NerdBank.GitVersioning.Tests/GitExtensionsTests.cs index bacae7e5..f8e07e03 100644 --- a/src/NerdBank.GitVersioning.Tests/GitExtensionsTests.cs +++ b/src/NerdBank.GitVersioning.Tests/GitExtensionsTests.cs @@ -438,14 +438,16 @@ public void GetVersionHeight_CachingPerf() // First calculation of height will not have the benefit of the cache, will be slow var (initialHeight, initialTime) = MeasureRuntime(() => this.Repo.Head.GetVersionHeight(repoRelativeSubDirectory), "get the initial (uncached) version height"); - // Second calculation of height should be able to use the cache file generated by the previous calculation - var (cachedHeight, cachedTime) = MeasureRuntime(() => this.Repo.Head.GetVersionHeight(repoRelativeSubDirectory), "get the version height for the unmodified repository (should be cached)"); + Assert.True(File.Exists(Path.Combine(RepoPath, repoRelativeSubDirectory, GitHeightCache.CacheFileName))); + + // Second calculation of height should be able to use the cache file generated by the previous calculation, even though it's for a child of the original path + var (cachedHeight, cachedTime) = MeasureRuntime(() => this.Repo.Head.GetVersionHeight(Path.Combine(repoRelativeSubDirectory, "new_sub_dir")), "get the version height for the unmodified repository (should be cached)"); // Want to see at least 20x perf increase Assert.InRange(cachedTime, TimeSpan.Zero, TimeSpan.FromTicks(initialTime.Ticks / 20)); Assert.Equal(initialHeight, numCommitsToTraverse + 1); Assert.Equal(initialHeight, cachedHeight); - + // Adding an additional commit and then getting the height should only involve walking a single commit this.Repo.Commit($"Final Test commit", this.Signer, this.Signer, new CommitOptions {AllowEmptyCommit = true}); // Third calculation of height should be able to use the cache file for the first set of commits but walk the last commit to determine the height @@ -455,6 +457,28 @@ public void GetVersionHeight_CachingPerf() Assert.InRange(cachedTime, TimeSpan.Zero, TimeSpan.FromTicks(initialTime.Ticks / 10)); } + [Fact] + public void GetVersionHeight_CachingMultipleVersions() + { + // TODO probably should make this test more vigorous + const string repoRelativeSubDirectory1 = "subdir1", repoRelativeSubDirectory2 = "subdir2";; + + this.WriteVersionFile( + new VersionOptions { Version = SemanticVersion.Parse("1.1") }, + repoRelativeSubDirectory1); + + this.WriteVersionFile( + new VersionOptions { Version = SemanticVersion.Parse("1.2") }, + repoRelativeSubDirectory2); + + // Verify that two separate cache files are generated + this.Repo.Head.GetVersionHeight(repoRelativeSubDirectory1); + Assert.True(File.Exists(Path.Combine(RepoPath, repoRelativeSubDirectory1, GitHeightCache.CacheFileName))); + + this.Repo.Head.GetVersionHeight(repoRelativeSubDirectory2); + Assert.True(File.Exists(Path.Combine(RepoPath, repoRelativeSubDirectory2, GitHeightCache.CacheFileName))); + } + [Fact] public void GetVersionHeight_CachingMultipleParents() { diff --git a/src/NerdBank.GitVersioning/GitExtensions.cs b/src/NerdBank.GitVersioning/GitExtensions.cs index 487d49bd..8f25647e 100644 --- a/src/NerdBank.GitVersioning/GitExtensions.cs +++ b/src/NerdBank.GitVersioning/GitExtensions.cs @@ -65,11 +65,11 @@ public static int GetVersionHeight(this Commit commit, string repoRelativeProjec if (versionHeightPosition.HasValue) { - var cache = new GitHeightCache(commit.GetRepository().Info.WorkingDirectory, repoRelativeProjectDirectory, baseSemVer.Version); + var cache = new GitHeightCache(commit.GetRepository().Info.WorkingDirectory, versionOptions.RelativeFilePath, baseVersion); + CachedHeight cachedHeight = null; if (useHeightCaching && cache.CachedHeightAvailable && (cachedHeight = cache.GetHeight()) != null) { - if (cachedHeight.CommitId.Equals(commit.Id)) // Cached height exactly matches the current commit return cachedHeight.Height; diff --git a/src/NerdBank.GitVersioning/GitHeightCache.cs b/src/NerdBank.GitVersioning/GitHeightCache.cs index 32544f91..4b04f246 100644 --- a/src/NerdBank.GitVersioning/GitHeightCache.cs +++ b/src/NerdBank.GitVersioning/GitHeightCache.cs @@ -19,7 +19,7 @@ namespace Nerdbank.GitVersioning /// public class GitHeightCache { - private readonly string repoRelativeProjectDirectory; + private readonly string versionFileRelativeDirectory; private readonly Version baseVersion; private static readonly JsonSerializer JsonSerializer = new JsonSerializer() @@ -43,17 +43,17 @@ public class GitHeightCache /// Creates a new height cache. /// /// The root path of the repository. - /// The relative path of the project within the repository. + /// The relative path of the version file within the repository to cache heights for. /// - public GitHeightCache(string repositoryPath, string repoRelativeProjectDirectory, Version baseVersion) + public GitHeightCache(string repositoryPath, string versionFileRelativePath, Version baseVersion) { - this.repoRelativeProjectDirectory = repoRelativeProjectDirectory; + this.versionFileRelativeDirectory = Path.GetDirectoryName(versionFileRelativePath); this.baseVersion = baseVersion; if (repositoryPath == null) this.heightCacheFilePath = null; else - this.heightCacheFilePath = Path.Combine(repositoryPath, repoRelativeProjectDirectory ?? "", CacheFileName); + this.heightCacheFilePath = Path.Combine(repositoryPath, this.versionFileRelativeDirectory ?? "", CacheFileName); this.cachedHeightAvailable = new Lazy(() => this.heightCacheFilePath != null && File.Exists(this.heightCacheFilePath)); } @@ -82,7 +82,7 @@ public CachedHeight GetHeight() return null; // Indicates that the project the cache is associated with has moved directories- any cached results may be invalid, so discard - if (cachedHeight.RelativeProjectDir != this.repoRelativeProjectDirectory) + if (cachedHeight.VersionRelativeDir != this.versionFileRelativeDirectory) return null; return cachedHeight; @@ -108,7 +108,7 @@ public void SetHeight(ObjectId commitId, int height) using (var jsonWriter = new JsonTextWriter(sw)) { jsonWriter.WriteComment("Cached commit height, created by Nerdbank.GitVersioning. Do not modify."); - JsonSerializer.Serialize(jsonWriter, new CachedHeight(commitId, height, this.baseVersion, this.repoRelativeProjectDirectory)); + JsonSerializer.Serialize(jsonWriter, new CachedHeight(commitId, height, this.baseVersion, this.versionFileRelativeDirectory)); } } @@ -133,13 +133,13 @@ public class CachedHeight /// /// /// - /// - public CachedHeight(ObjectId commitId, int height, Version baseVersion, string relativeProjectDir) + /// + public CachedHeight(ObjectId commitId, int height, Version baseVersion, string versionRelativeDir) { this.CommitId = commitId; this.Height = height; this.BaseVersion = baseVersion; - this.RelativeProjectDir = relativeProjectDir; + this.VersionRelativeDir = versionRelativeDir; } /// @@ -158,9 +158,9 @@ public CachedHeight(ObjectId commitId, int height, Version baseVersion, string r public ObjectId CommitId { get; } /// - /// The relative path of the project this cached height belong to. + /// The relative directory path for the version file that this cached is associated with. /// - public string RelativeProjectDir { get; } + public string VersionRelativeDir { get; } /// public override string ToString() => $"({CommitId}, {Height}, {BaseVersion})"; diff --git a/src/NerdBank.GitVersioning/VersionFile.cs b/src/NerdBank.GitVersioning/VersionFile.cs index c8745e04..a3216b8b 100644 --- a/src/NerdBank.GitVersioning/VersionFile.cs +++ b/src/NerdBank.GitVersioning/VersionFile.cs @@ -52,7 +52,7 @@ public static VersionOptions GetVersion(LibGit2Sharp.Commit commit, string repoR var versionTxtBlob = commit.Tree[candidatePath]?.Target as LibGit2Sharp.Blob; if (versionTxtBlob != null) { - var result = TryReadVersionFile(new StreamReader(versionTxtBlob.GetContentStream())); + var result = TryReadVersionFile(new StreamReader(versionTxtBlob.GetContentStream()), candidatePath); if (result != null) { return result; @@ -72,7 +72,7 @@ public static VersionOptions GetVersion(LibGit2Sharp.Commit commit, string repoR VersionOptions result; try { - result = TryReadVersionJsonContent(versionJsonContent, searchDirectory); + result = TryReadVersionJsonContent(versionJsonContent, searchDirectory, candidatePath); } catch (FormatException ex) { @@ -158,7 +158,7 @@ public static VersionOptions GetVersion(string projectDirectory, out string actu { using (var sr = new StreamReader(File.OpenRead(versionTxtPath))) { - var result = TryReadVersionFile(sr); + var result = TryReadVersionFile(sr, repo?.GetRepoRelativePath(versionTxtPath)); if (result != null) { actualDirectory = searchDirectory; @@ -174,7 +174,7 @@ public static VersionOptions GetVersion(string projectDirectory, out string actu var repoRelativeBaseDirectory = repo?.GetRepoRelativePath(searchDirectory); VersionOptions result = - TryReadVersionJsonContent(versionJsonContent, repoRelativeBaseDirectory); + TryReadVersionJsonContent(versionJsonContent, repoRelativeBaseDirectory, repo?.GetRepoRelativePath(versionJsonPath)); if (result?.Inherit ?? false) { if (parentDirectory != null) @@ -314,7 +314,7 @@ public static string SetVersion(string projectDirectory, Version version, string /// /// The content of the version.txt file to read. /// The version information read from the file; or null if a deserialization error occurs. - private static VersionOptions TryReadVersionFile(TextReader versionTextContent) + private static VersionOptions TryReadVersionFile(TextReader versionTextContent, string candidatePath) { string versionLine = versionTextContent.ReadLine(); string prereleaseVersion = versionTextContent.ReadLine(); @@ -332,6 +332,7 @@ private static VersionOptions TryReadVersionFile(TextReader versionTextContent) return new VersionOptions { Version = semVer, + RelativeFilePath = candidatePath }; } @@ -341,11 +342,13 @@ private static VersionOptions TryReadVersionFile(TextReader versionTextContent) /// The content of the version.json file. /// Directory that this version.json file is relative to the root of the repository. /// The deserialized object, if deserialization was successful. - private static VersionOptions TryReadVersionJsonContent(string jsonContent, string repoRelativeBaseDirectory) + private static VersionOptions TryReadVersionJsonContent(string jsonContent, string repoRelativeBaseDirectory, string candidatePath) { try { - return JsonConvert.DeserializeObject(jsonContent, VersionOptions.GetJsonSettings(repoRelativeBaseDirectory: repoRelativeBaseDirectory)); + var result = JsonConvert.DeserializeObject(jsonContent, VersionOptions.GetJsonSettings(repoRelativeBaseDirectory: repoRelativeBaseDirectory)); + result.RelativeFilePath = candidatePath; + return result; } catch (JsonSerializationException) { diff --git a/src/NerdBank.GitVersioning/VersionOptions.cs b/src/NerdBank.GitVersioning/VersionOptions.cs index f2878a7f..465de11d 100644 --- a/src/NerdBank.GitVersioning/VersionOptions.cs +++ b/src/NerdBank.GitVersioning/VersionOptions.cs @@ -253,6 +253,13 @@ public int VersionHeightOffsetOrDefault [JsonProperty(DefaultValueHandling = DefaultValueHandling.Ignore)] public bool Inherit { get; set; } + /// + /// The relative path of the source file (if deserialized from a file), otherwise null. + /// If is true, returns the path of the child version file. + /// + [JsonIgnore] + public string RelativeFilePath { get; set; } + /// /// Gets the position in a computed version that the version height should appear. ///