UPath performance improvements #77
Conversation
| @@ -67,6 +70,16 @@ public void TestEquals() | |||
| Assert.False(pathInfo.Equals(null)); | |||
| } | |||
|
|
|||
| [Fact] | |||
| public void TestIsNullAndEmpty() | |||
There was a problem hiding this comment.
Empty did not have any tests. Moved IsNull to here from the test below
| @@ -401,7 +401,7 @@ public static string[] ReadAllLines(this IFileSystem fs, UPath path) | |||
| using (var reader = new StreamReader(stream)) | |||
| { | |||
| var lines = new List<string>(); | |||
| string line; | |||
| string? line; | |||
| @@ -601,8 +601,7 @@ public static IEnumerable<UPath> EnumerateDirectories(this IFileSystem fileSyste | |||
| public static IEnumerable<UPath> 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); | |||
There was a problem hiding this comment.
avoids enumerator allocation
| Throw(this.GetType()); | ||
| } | ||
|
|
||
| static void Throw(Type type) |
There was a problem hiding this comment.
Moving the trowing of the exception out of the method, allows the method to be inlined.
| { | ||
| dotCount++; | ||
| } | ||
| else if (c == DirectorySeparator || c == '\\') |
There was a problem hiding this comment.
added elseif, as it can only be either a dot or a separator
| lastIndex = i; | ||
| if (dotCount > 0 && // has dots | ||
| dotCount == part.Length && // only dots | ||
| (dotCount != 2 || parts.Count > 1)) // Skip ".." if it's the first part |
There was a problem hiding this comment.
I made that logic a bit more complex, to only really set processParts, when really needed, so more paths are using the fast path only.
| @@ -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); | |||
There was a problem hiding this comment.
Substring should be ok to use no validation right?
| if (path.FullName is null) | ||
| throw new ArgumentNullException(name); | ||
| return path; | ||
| if (path.IsNull) |
There was a problem hiding this comment.
FullName is null -> IsNull and the local method to move out the exception to help inline it.
src/Zio/UPathExtensions.cs
Outdated
| /// <exception cref="System.ArgumentException">If the path is not absolute using the parameter name from <paramref name="name"/></exception> | ||
| public static UPath AssertAbsolute(this UPath path, string name = "path") | ||
| public static void AssertAbsolute(this UPath path, string name = "path") |
There was a problem hiding this comment.
Removed the return, as it created a new UPath where only one occurrance actually used it. (New Upath went through the normalization method, which is rather expensive when unused.
|
Thanks, I'm ok with the changes. |
From here: #77 (comment) Can you let me know what you prefer? |
|
Reminder: Which way shall I adjust it to fix the tests? |
|
I'm on a business trip and pretty busy, will come back hopefully next week, my apologies. |
|
The CI is failing because |
|
I had to make further change to main (drop some old targets, but that's ok) |
|
rebased and solved the failing tests, should be ready. |
|
Thanks for the contribution, my apologies that it took some time and ping/pong to review it. 🙂 |
In my project, we're using your great library.
Lately we've been analyzing our performance, and your library came up a few times. This collection of fixes were mostly some low hanging fruits to improve things here and there a bit. I'll comment on them for better understanding.
There are a few other places that I can improve on, but that's for later if wanted.