From c2c62bdecaec7747c8c91f8df441848898072249 Mon Sep 17 00:00:00 2001 From: Sandro Bollhalder Date: Fri, 27 Oct 2023 18:56:34 +0200 Subject: [PATCH] UPath performance improvements (#77) * UPath performance improvements * revert public API changes * missed the documentation tags * restore exception behaviour --------- Co-authored-by: Sandro Bollhalder --- src/Zio.Tests/TestUPath.cs | 16 +- src/Zio/FileSystemExtensions.cs | 10 +- src/Zio/FileSystems/FileSystem.cs | 7 +- src/Zio/FileSystems/PhysicalFileSystem.cs | 8 +- src/Zio/UPath.cs | 267 ++++++++++------------ src/Zio/UPathExtensions.cs | 21 +- 6 files changed, 161 insertions(+), 168 deletions(-) diff --git a/src/Zio.Tests/TestUPath.cs b/src/Zio.Tests/TestUPath.cs index 81ab331..0b19857 100644 --- a/src/Zio.Tests/TestUPath.cs +++ b/src/Zio.Tests/TestUPath.cs @@ -41,6 +41,9 @@ public class TestUPath [InlineData("...a/b../", "...a/b..")] [InlineData("...a/..", "")] [InlineData("...a/b/..", "...a")] + [InlineData("..a/b", "..a/b")] + [InlineData("c/..d", "c/..d")] + [InlineData("c/d..", "c/d..")] public void TestNormalize(string pathAsText, string expectedResult) { var path = new UPath(pathAsText); @@ -67,6 +70,16 @@ public void TestEquals() Assert.False(pathInfo.Equals(null)); } + [Fact] + public void TestIsNullAndEmpty() + { + Assert.True(default(UPath).IsNull); + Assert.False(default(UPath).IsEmpty); + Assert.True(new UPath("").IsEmpty); + Assert.False(new UPath("").IsNull); + Assert.False(new UPath("/").IsEmpty); + } + [Fact] public void TestAbsoluteAndRelative() { @@ -83,9 +96,6 @@ public void TestAbsoluteAndRelative() Assert.True(path.IsAbsolute); Assert.Equal(path, path.ToAbsolute()); - - path = new UPath(); - Assert.True(path.IsNull); } [Theory] diff --git a/src/Zio/FileSystemExtensions.cs b/src/Zio/FileSystemExtensions.cs index a162ae2..49594bd 100644 --- a/src/Zio/FileSystemExtensions.cs +++ b/src/Zio/FileSystemExtensions.cs @@ -401,7 +401,7 @@ public static string[] ReadAllLines(this IFileSystem fs, UPath path) using (var reader = new StreamReader(stream)) { var lines = new List(); - string line; + string? line; while ((line = reader.ReadLine()) != null) { lines.Add(line); @@ -430,7 +430,7 @@ public static string[] ReadAllLines(this IFileSystem fs, UPath path, Encoding en using (var reader = new StreamReader(stream, encoding)) { var lines = new List(); - string line; + string? line; while ((line = reader.ReadLine()) != null) { lines.Add(line); @@ -601,8 +601,7 @@ public static IEnumerable EnumerateDirectories(this IFileSystem fileSyste public static IEnumerable EnumerateDirectories(this IFileSystem fileSystem, UPath path, string searchPattern, SearchOption searchOption) { if (searchPattern is null) throw new ArgumentNullException(nameof(searchPattern)); - foreach (var subPath in fileSystem.EnumeratePaths(path, searchPattern, searchOption, SearchTarget.Directory)) - yield return subPath; + return fileSystem.EnumeratePaths(path, searchPattern, searchOption, SearchTarget.Directory); } /// @@ -644,8 +643,7 @@ public static IEnumerable EnumerateFiles(this IFileSystem fileSystem, UPa public static IEnumerable EnumerateFiles(this IFileSystem fileSystem, UPath path, string searchPattern, SearchOption searchOption) { if (searchPattern is null) throw new ArgumentNullException(nameof(searchPattern)); - foreach (var subPath in fileSystem.EnumeratePaths(path, searchPattern, searchOption, SearchTarget.File)) - yield return subPath; + return fileSystem.EnumeratePaths(path, searchPattern, searchOption, SearchTarget.File); } /// diff --git a/src/Zio/FileSystems/FileSystem.cs b/src/Zio/FileSystems/FileSystem.cs index 5993de3..0f7feb0 100644 --- a/src/Zio/FileSystems/FileSystem.cs +++ b/src/Zio/FileSystems/FileSystem.cs @@ -612,7 +612,12 @@ private void AssertNotDisposed() { if (IsDisposing || IsDisposed) { - throw new ObjectDisposedException($"This instance `{GetType()}` is already disposed."); + Throw(this.GetType()); + } + + static void Throw(Type type) + { + throw new ObjectDisposedException($"This instance `{type}` is already disposed."); } } diff --git a/src/Zio/FileSystems/PhysicalFileSystem.cs b/src/Zio/FileSystems/PhysicalFileSystem.cs index 25dc45b..d393723 100644 --- a/src/Zio/FileSystems/PhysicalFileSystem.cs +++ b/src/Zio/FileSystems/PhysicalFileSystem.cs @@ -815,9 +815,7 @@ protected override string ConvertPathToInternalImpl(UPath path) if (absolutePath.Length > DrivePrefixOnWindows.Length + 1) builder.Append(absolutePath.Replace(UPath.DirectorySeparator, '\\').Substring(DrivePrefixOnWindows.Length + 2)); - var result = builder.ToString(); - builder.Length = 0; - return result; + return builder.ToString(); } return absolutePath; } @@ -841,9 +839,7 @@ protected override UPath ConvertPathFromInternalImpl(string innerPath) if (absolutePath.Length > 2) builder.Append(absolutePath.Substring(2)); - var result = builder.ToString(); - builder.Length = 0; - return new UPath(result); + return new UPath(builder.ToString()); } return innerPath; } diff --git a/src/Zio/UPath.cs b/src/Zio/UPath.cs index 0cc17c4..f5f98cb 100644 --- a/src/Zio/UPath.cs +++ b/src/Zio/UPath.cs @@ -2,7 +2,6 @@ // This file is licensed under the BSD-Clause 2 license. // See the license.txt file in the project root for more information. -using System.Diagnostics; using System.Text; namespace Zio; @@ -83,13 +82,13 @@ internal UPath(string path, bool safe) /// Gets a value indicating whether this path is empty ( equals to the empty string) /// /// true if this instance is empty; otherwise, false. - public bool IsEmpty => FullName == string.Empty; + public bool IsEmpty => this.FullName?.Length == 0; /// /// Gets a value indicating whether this path is absolute by starting with a leading `/`. /// /// true if this path is absolute; otherwise, false. - public bool IsAbsolute => FullName?.StartsWith("/", StringComparison.Ordinal) ?? false; + public bool IsAbsolute => FullName is not null && FullName.Length > 0 && FullName[0] == '/'; /// /// Gets a value indicating whether this path is relative by **not** starting with a leading `/`. @@ -137,31 +136,13 @@ public static UPath Combine(UPath path1, UPath path2) if (path2.FullName is null) throw new ArgumentNullException(nameof(path2)); - if (path1.IsEmpty && path2.IsEmpty) - return Empty; - // If the right path is absolute, it takes priority over path1 - if (path2.IsAbsolute) + if (path1.IsEmpty || path2.IsAbsolute) return path2; - var builder = InternalHelperTls.Builder; - Debug.Assert(builder.Length == 0); - - if (!path1.IsEmpty) - { - builder.Append(path1.FullName); - builder.Append('/'); - } - - if (!path2.IsEmpty) - builder.Append(path2.FullName); - try { - var newPath = builder.ToString(); - // Make sure to clean the builder as it is going to be when creating a new UPath - builder.Length = 0; - return new UPath(newPath); + return new UPath($"{path1.FullName}/{path2.FullName}"); } catch (ArgumentException ex) { @@ -256,7 +237,7 @@ public override string ToString() /// Tries to parse the specified string into a /// /// The path as a string. - /// The path parsed if successfull. + /// The path parsed if successful. /// true if path was parsed successfully, false otherwise. public static bool TryParse(string path, out UPath pathInfo) { @@ -296,165 +277,159 @@ internal static StringBuilder GetSharedStringBuilder() var parts = internalHelper.Slices; parts.Clear(); - var builder = internalHelper.Builder; - builder.Length = 0; - var lastIndex = 0; - try + var i = 0; + var processParts = false; + var dotCount = 0; + for (; i < path.Length; i++) { - var i = 0; - var processParts = false; - var dotCount = 0; - for (; i < path.Length; i++) - { - var c = path[i]; - - // We don't disallow characters, as we let the IFileSystem implementations decided for them - // depending on the platform + var c = path[i]; - //if (c < ' ' || c == ':' || c == '<' || c == '>' || c == '"' || c == '|') - //{ - // throw new InvalidUPathException($"The path `{path}` contains invalid characters `{c}`"); - //} + // We don't disallow characters, as we let the IFileSystem implementations decided for them + // depending on the platform - if (c == '.') - dotCount++; + //if (c < ' ' || c == ':' || c == '<' || c == '>' || c == '"' || c == '|') + //{ + // throw new InvalidUPathException($"The path `{path}` contains invalid characters `{c}`"); + //} - if (c == DirectorySeparator || c == '\\') - { - // optimization: If we don't expect to process the path - // and we only have a trailing / or \\, then just perform - // a substring on the path - if (!processParts && i + 1 == path.Length) - return path.Substring(0, path.Length - 1); + if (c == '.') + { + dotCount++; + } + else if (c == DirectorySeparator || c == '\\') + { + // optimization: If we don't expect to process the path + // and we only have a trailing / or \\, then just perform + // a substring on the path + if (!processParts && i + 1 == path.Length) + return path.Substring(0, path.Length - 1); - if (c == '\\') - processParts = true; + if (c == '\\') + processParts = true; - var endIndex = i - 1; - for (i++; i < path.Length; i++) + var endIndex = i - 1; + for (i++; i < path.Length; i++) + { + c = path[i]; + if (c == DirectorySeparator || c == '\\') { - c = path[i]; - if (c == DirectorySeparator || c == '\\') - { - // If we have consecutive / or \\, we need to process parts - processParts = true; - continue; - } - break; + // If we have consecutive / or \\, we need to process parts + processParts = true; + continue; } + break; + } - if (endIndex >= lastIndex || endIndex == -1) - { - var part = new TextSlice(lastIndex, endIndex); - parts.Add(part); + if (endIndex >= lastIndex || endIndex == -1) + { + var part = new TextSlice(lastIndex, endIndex); + parts.Add(part); - // If the previous part had only dots, we need to process it - if (part.Length == dotCount) - processParts = true; - } - dotCount = c == '.' ? 1 : 0; - lastIndex = i; + if (dotCount > 0 && // has dots + dotCount == part.Length && // only dots + (dotCount != 2 || parts.Count > 1)) // Skip ".." if it's the first part + processParts = true; } + dotCount = c == '.' ? 1 : 0; + lastIndex = i; } + } - if (lastIndex < path.Length) - { - var part = new TextSlice(lastIndex, path.Length - 1); - parts.Add(part); + if (lastIndex < path.Length) + { + var part = new TextSlice(lastIndex, path.Length - 1); + parts.Add(part); - // If the previous part had only dots, we need to process it - if (part.Length == dotCount) - processParts = true; - } + // If the previous part had only dots, we need to process it + if (part.Length == dotCount) + processParts = true; + } - // Optimized path if we don't need to compact the path - if (!processParts) - return path; + // Optimized path if we don't need to compact the path + if (!processParts) + return path; - // Slow path, we need to process the parts - for (i = 0; i < parts.Count; i++) - { - var part = parts[i]; - var partLength = part.Length; - if (partLength < 1) - continue; + // Slow path, we need to process the parts + for (i = 0; i < parts.Count; i++) + { + var part = parts[i]; + var partLength = part.Length; + if (partLength < 1) + continue; + + if (path[part.Start] != '.') + continue; - if (path[part.Start] != '.') + if (partLength == 1) + { + // We have a '.' + if (parts.Count > 1) + parts.RemoveAt(i--); + } + else + { + if (path[part.Start + 1] != '.') continue; - if (partLength == 1) + // Throws an exception if our slice parth contains only `.` and is longer than 2 characters + if (partLength > 2) { - // We have a '.' - if (parts.Count > 1) - parts.RemoveAt(i--); - } - else - { - if (path[part.Start + 1] != '.') - continue; - - // Throws an exception if our slice parth contains only `.` and is longer than 2 characters - if (partLength > 2) + var isValid = false; + for (var j = part.Start + 2; j <= part.End; j++) { - var isValid = false; - for (var j = part.Start + 2; j <= part.End; j++) + if (path[j] != '.') { - if (path[j] != '.') - { - isValid = true; - break; - } - } - - if (!isValid) - { - errorMessage = $"The path `{path}` contains invalid dots `{path.Substring(part.Start, part.Length)}` while only `.` or `..` are supported"; - return string.Empty; + isValid = true; + break; } + } - // Otherwise, it is a valid path part - continue; + if (!isValid) + { + errorMessage = $"The path `{path}` contains invalid dots `{path.Substring(part.Start, part.Length)}` while only `.` or `..` are supported"; + return string.Empty; } - if (i - 1 >= 0) + // Otherwise, it is a valid path part + continue; + } + + if (i - 1 >= 0) + { + var previousSlice = parts[i - 1]; + if (!IsDotDot(previousSlice, path)) { - var previousSlice = parts[i - 1]; - if (!IsDotDot(previousSlice, path)) + if (previousSlice.Length == 0) { - if (previousSlice.Length == 0) - { - errorMessage = $"The path `{path}` cannot go to the parent (..) of a root path /"; - return string.Empty; - } - parts.RemoveAt(i--); - parts.RemoveAt(i--); + errorMessage = $"The path `{path}` cannot go to the parent (..) of a root path /"; + return string.Empty; } + parts.RemoveAt(i--); + parts.RemoveAt(i--); } } } + } - // If we have a single part and it is empty, it is a root - if (parts.Count == 1 && parts[0].Start == 0 && parts[0].End < 0) - { - return "/"; - } - - for (i = 0; i < parts.Count; i++) - { - var slice = parts[i]; - if (slice.Length > 0) - builder.Append(path, slice.Start, slice.Length); - if (i + 1 < parts.Count) - builder.Append('/'); - } - return builder.ToString(); + // If we have a single part and it is empty, it is a root + if (parts.Count == 1 && parts[0].Start == 0 && parts[0].End < 0) + { + return "/"; } - finally + + + var builder = internalHelper.Builder; + builder.Length = 0; + for (i = 0; i < parts.Count; i++) { - parts.Clear(); - builder.Length = 0; + var slice = parts[i]; + if (slice.Length > 0) + builder.Append(path, slice.Start, slice.Length); + if (i + 1 < parts.Count) + builder.Append('/'); } + return builder.ToString(); } private static bool IsDotDot(TextSlice slice, string path) diff --git a/src/Zio/UPathExtensions.cs b/src/Zio/UPathExtensions.cs index bef2f73..5aa8c2b 100644 --- a/src/Zio/UPathExtensions.cs +++ b/src/Zio/UPathExtensions.cs @@ -67,7 +67,7 @@ public static UPath GetDirectory(this UPath path) var lastIndex = fullname.LastIndexOf(UPath.DirectorySeparator); if (lastIndex > 0) { - return fullname.Substring(0, lastIndex); + return new UPath(fullname.Substring(0, lastIndex), true); } return lastIndex == 0 ? UPath.Root : UPath.Empty; } @@ -243,9 +243,12 @@ public static bool IsInDirectory(this UPath path, UPath directory, bool recursiv /// If the path was null using the parameter name from public static UPath AssertNotNull(this UPath path, string name = "path") { - if (path.FullName is null) - throw new ArgumentNullException(name); + if (path.IsNull) + Throw(name); + return path; + + static void Throw(string name) => throw new ArgumentNullException(name); } /// @@ -257,10 +260,16 @@ public static UPath AssertNotNull(this UPath path, string name = "path") /// If the path is not absolute using the parameter name from public static UPath AssertAbsolute(this UPath path, string name = "path") { - AssertNotNull(path, name); - if (!path.IsAbsolute) + Throw(path, name); + + return path; + + static void Throw(UPath path, string name) + { + // Assert not null first, as a not absolute path could also be null, and if so, an ArgumentNullException shall be thrown + path.AssertNotNull(name); throw new ArgumentException($"Path `{path}` must be absolute", name); - return path.FullName; + } } }