Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

UpdateIndexOfAny calls with invalid path/filename toSearchValues<char> for more efficient char searching#24896

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.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

ArmaanMcleod
Copy link
Contributor

@ArmaanMcleodArmaanMcleod commentedJan 29, 2025•
edited
Loading

PR Summary

Similar to the below PRs:

#24880
#24879

I've replaced the calls usingIndexOfAny withPath.GetInvalidFileNameChars() &Path.GetInvalidPathChars() toSearchValues<char>. I put the methods inPathUtils which caches the search values and reuses them acrossCommandSearcher,InitialSessionSate,ModuleCmdletBase andFileSystemProvider classes.

This is because these char arrays have quite a bit of characters to scan and from benchmarks it seems search values is performing a lot better as shown below in BenchmarkDotnet.

Benchmark

Details

Test Code

usingSystem.Buffers;usingBenchmarkDotNet.Attributes;usingBenchmarkDotNet.Configs;usingBenchmarkDotNet.Order;usingBenchmarkDotNet.Running;namespaceBenchMark;[MemoryDiagnoser][RankColumn][Orderer(SummaryOrderPolicy.FastestToSlowest)][GroupBenchmarksBy(BenchmarkLogicalGroupRule.ByCategory,BenchmarkLogicalGroupRule.ByParams)]publicclassSearchValuesBenchmark{publicstaticreadonlyList<string>Paths=[];privatestaticreadonlySearchValues<char>invalidPathChars=SearchValues.Create(Path.GetInvalidPathChars());privatestaticreadonlychar[]invalidPathCharsArray=Path.GetInvalidPathChars();privatestaticreadonlySearchValues<char>invalidFileNameChars=SearchValues.Create(Path.GetInvalidFileNameChars());privatestaticreadonlychar[]invalidFileNameCharsArray=Path.GetInvalidFileNameChars();[Params(10,100,1000)]publicintPathCount;[GlobalSetup]publicvoidSetup(){string[]directories=["Documents","Downloads","Music","Pictures","Videos"];string[]fileExtensions=[".txt",".jpg",".mp3",".mp4",".pdf"];for(inti=1;i<=PathCount;i++){foreach(stringdirindirectories){foreach(stringextinfileExtensions){stringpath=$@"C:\Users\TestUser\{dir}\file{i}{ext}";Paths.Add(path);}}}}[Benchmark][BenchmarkCategory("InvalidPathChars")]publicintWithSearchValuesInvalidPathChars(){intinvalidCount=0;foreach(varpathinPaths){if(path.AsSpan().ContainsAny(invalidPathChars)){invalidCount++;}}returninvalidCount;}[Benchmark(Baseline=true)][BenchmarkCategory("InvalidPathChars")]publicintWithIndexOfAnyInvalidPathChars(){intinvalidCount=0;foreach(varpathinPaths){if(path.AsSpan().IndexOfAny(invalidPathCharsArray)>=0){invalidCount++;}}returninvalidCount;}[Benchmark][BenchmarkCategory("InvalidFileNameChars")]publicintWithSearchValuesInvalidFileNameChars(){intinvalidCount=0;foreach(varpathinPaths){varroot=Path.GetPathRoot(path);varpathWithoutDriveRoot=string.IsNullOrEmpty(root)?path:path[root.Length..];foreach(varsegmentinpathWithoutDriveRoot.Split(Path.DirectorySeparatorChar)){if(segment.AsSpan().ContainsAny(invalidFileNameChars)){invalidCount++;}}}returninvalidCount;}[Benchmark(Baseline=true)][BenchmarkCategory("InvalidFileNameChars")]publicintWithIndexOfAnyInvalidFileNameChars(){intinvalidCount=0;foreach(varpathinPaths){varroot=Path.GetPathRoot(path);varpathWithoutDriveRoot=string.IsNullOrEmpty(root)?path:path[root.Length..];foreach(varsegmentinpathWithoutDriveRoot.Split(Path.DirectorySeparatorChar)){if(segment.AsSpan().IndexOfAny(invalidFileNameCharsArray)>=0){invalidCount++;}}}returninvalidCount;}}publicclassProgram{publicstaticvoidMain(string[]args){BenchmarkRunner.Run<SearchValuesBenchmark>();}}

Results

// * Detailed results *SearchValuesBenchmark.WithSearchValuesInvalidFileNameChars: DefaultJob [PathCount=10]Runtime = .NET 9.0.1 (9.0.124.61010), X64 RyuJIT AVX2; GC = Concurrent WorkstationMean = 33.130 us, StdErr = 0.157 us (0.47%), N = 15, StdDev = 0.608 usMin = 32.332 us, Q1 = 32.742 us, Median = 33.019 us, Q3 = 33.516 us, Max = 34.403 usIQR = 0.773 us, LowerFence = 31.582 us, UpperFence = 34.675 usConfidenceInterval = [32.480 us; 33.780 us] (CI 99.9%), Margin = 0.650 us (1.96% of Mean)Skewness = 0.55, Kurtosis = 2.1, MValue = 2-------------------- Histogram --------------------[32.008 us ; 33.252 us) | @@@@@@@@@@[33.252 us ; 33.974 us) | @@@@[33.974 us ; 34.726 us) | @---------------------------------------------------SearchValuesBenchmark.WithIndexOfAnyInvalidFileNameChars: DefaultJob [PathCount=10]Runtime = .NET 9.0.1 (9.0.124.61010), X64 RyuJIT AVX2; GC = Concurrent WorkstationMean = 49.970 us, StdErr = 0.236 us (0.47%), N = 15, StdDev = 0.915 usMin = 48.193 us, Q1 = 49.362 us, Median = 49.957 us, Q3 = 50.514 us, Max = 51.652 usIQR = 1.152 us, LowerFence = 47.634 us, UpperFence = 52.242 usConfidenceInterval = [48.992 us; 50.949 us] (CI 99.9%), Margin = 0.978 us (1.96% of Mean)Skewness = 0.08, Kurtosis = 2.27, MValue = 2-------------------- Histogram --------------------[48.177 us ; 49.297 us) | @@@[49.297 us ; 51.077 us) | @@@@@@@@@@[51.077 us ; 52.139 us) | @@---------------------------------------------------SearchValuesBenchmark.WithSearchValuesInvalidFileNameChars: DefaultJob [PathCount=100]Runtime = .NET 9.0.1 (9.0.124.61010), X64 RyuJIT AVX2; GC = Concurrent WorkstationMean = 300.889 us, StdErr = 1.504 us (0.50%), N = 19, StdDev = 6.554 usMin = 291.971 us, Q1 = 296.737 us, Median = 299.052 us, Q3 = 304.806 us, Max = 316.375 usIQR = 8.069 us, LowerFence = 284.634 us, UpperFence = 316.909 usConfidenceInterval = [294.992 us; 306.785 us] (CI 99.9%), Margin = 5.896 us (1.96% of Mean)Skewness = 0.73, Kurtosis = 2.53, MValue = 2-------------------- Histogram --------------------[288.748 us ; 294.847 us) | @@[294.847 us ; 303.910 us) | @@@@@@@@@@@[303.910 us ; 313.151 us) | @@@@@[313.151 us ; 319.598 us) | @---------------------------------------------------SearchValuesBenchmark.WithIndexOfAnyInvalidFileNameChars: DefaultJob [PathCount=100]Runtime = .NET 9.0.1 (9.0.124.61010), X64 RyuJIT AVX2; GC = Concurrent WorkstationMean = 470.042 us, StdErr = 1.704 us (0.36%), N = 15, StdDev = 6.601 usMin = 461.306 us, Q1 = 464.105 us, Median = 469.228 us, Q3 = 474.797 us, Max = 481.591 usIQR = 10.692 us, LowerFence = 448.067 us, UpperFence = 490.836 usConfidenceInterval = [462.985 us; 477.099 us] (CI 99.9%), Margin = 7.057 us (1.50% of Mean)Skewness = 0.22, Kurtosis = 1.67, MValue = 2-------------------- Histogram --------------------[458.736 us ; 485.104 us) | @@@@@@@@@@@@@@@---------------------------------------------------SearchValuesBenchmark.WithSearchValuesInvalidFileNameChars: DefaultJob [PathCount=1000]Runtime = .NET 9.0.1 (9.0.124.61010), X64 RyuJIT AVX2; GC = Concurrent WorkstationMean = 3.245 ms, StdErr = 0.017 ms (0.51%), N = 21, StdDev = 0.076 msMin = 3.110 ms, Q1 = 3.207 ms, Median = 3.234 ms, Q3 = 3.298 ms, Max = 3.372 msIQR = 0.090 ms, LowerFence = 3.072 ms, UpperFence = 3.433 msConfidenceInterval = [3.182 ms; 3.309 ms] (CI 99.9%), Margin = 0.064 ms (1.96% of Mean)Skewness = 0.07, Kurtosis = 2, MValue = 2-------------------- Histogram --------------------[3.089 ms ; 3.161 ms) | @@@[3.161 ms ; 3.249 ms) | @@@@@@@@@@[3.249 ms ; 3.385 ms) | @@@@@@@@---------------------------------------------------SearchValuesBenchmark.WithIndexOfAnyInvalidFileNameChars: DefaultJob [PathCount=1000]Runtime = .NET 9.0.1 (9.0.124.61010), X64 RyuJIT AVX2; GC = Concurrent WorkstationMean = 5.183 ms, StdErr = 0.021 ms (0.40%), N = 15, StdDev = 0.080 msMin = 4.977 ms, Q1 = 5.157 ms, Median = 5.189 ms, Q3 = 5.224 ms, Max = 5.308 msIQR = 0.067 ms, LowerFence = 5.056 ms, UpperFence = 5.325 msConfidenceInterval = [5.097 ms; 5.268 ms] (CI 99.9%), Margin = 0.085 ms (1.64% of Mean)Skewness = -0.86, Kurtosis = 3.68, MValue = 2-------------------- Histogram --------------------[4.935 ms ; 5.063 ms) | @[5.063 ms ; 5.351 ms) | @@@@@@@@@@@@@@---------------------------------------------------SearchValuesBenchmark.WithSearchValuesInvalidPathChars: DefaultJob [PathCount=10]Runtime = .NET 9.0.1 (9.0.124.61010), X64 RyuJIT AVX2; GC = Concurrent WorkstationMean = 1.122 us, StdErr = 0.002 us (0.21%), N = 15, StdDev = 0.009 usMin = 1.110 us, Q1 = 1.115 us, Median = 1.121 us, Q3 = 1.128 us, Max = 1.137 usIQR = 0.013 us, LowerFence = 1.095 us, UpperFence = 1.149 usConfidenceInterval = [1.112 us; 1.132 us] (CI 99.9%), Margin = 0.010 us (0.87% of Mean)Skewness = 0.36, Kurtosis = 1.7, MValue = 2-------------------- Histogram --------------------[1.105 us ; 1.142 us) | @@@@@@@@@@@@@@@---------------------------------------------------SearchValuesBenchmark.WithIndexOfAnyInvalidPathChars: DefaultJob [PathCount=10]Runtime = .NET 9.0.1 (9.0.124.61010), X64 RyuJIT AVX2; GC = Concurrent WorkstationMean = 8.156 us, StdErr = 0.022 us (0.27%), N = 15, StdDev = 0.084 usMin = 8.028 us, Q1 = 8.098 us, Median = 8.139 us, Q3 = 8.203 us, Max = 8.343 usIQR = 0.105 us, LowerFence = 7.940 us, UpperFence = 8.361 usConfidenceInterval = [8.066 us; 8.246 us] (CI 99.9%), Margin = 0.090 us (1.10% of Mean)Skewness = 0.6, Kurtosis = 2.5, MValue = 2-------------------- Histogram --------------------[7.983 us ; 8.190 us) | @@@@@@@@@@@[8.190 us ; 8.388 us) | @@@@---------------------------------------------------SearchValuesBenchmark.WithSearchValuesInvalidPathChars: DefaultJob [PathCount=100]Runtime = .NET 9.0.1 (9.0.124.61010), X64 RyuJIT AVX2; GC = Concurrent WorkstationMean = 10.059 us, StdErr = 0.030 us (0.29%), N = 15, StdDev = 0.115 usMin = 9.901 us, Q1 = 9.964 us, Median = 10.055 us, Q3 = 10.136 us, Max = 10.254 usIQR = 0.172 us, LowerFence = 9.707 us, UpperFence = 10.394 usConfidenceInterval = [9.936 us; 10.182 us] (CI 99.9%), Margin = 0.123 us (1.22% of Mean)Skewness = 0.17, Kurtosis = 1.64, MValue = 2-------------------- Histogram --------------------[9.840 us ; 10.315 us) | @@@@@@@@@@@@@@@---------------------------------------------------SearchValuesBenchmark.WithIndexOfAnyInvalidPathChars: DefaultJob [PathCount=100]Runtime = .NET 9.0.1 (9.0.124.61010), X64 RyuJIT AVX2; GC = Concurrent WorkstationMean = 80.813 us, StdErr = 0.218 us (0.27%), N = 15, StdDev = 0.845 usMin = 79.503 us, Q1 = 80.224 us, Median = 80.867 us, Q3 = 81.378 us, Max = 82.684 usIQR = 1.155 us, LowerFence = 78.492 us, UpperFence = 83.110 usConfidenceInterval = [79.909 us; 81.716 us] (CI 99.9%), Margin = 0.903 us (1.12% of Mean)Skewness = 0.31, Kurtosis = 2.42, MValue = 2-------------------- Histogram --------------------[79.053 us ; 83.134 us) | @@@@@@@@@@@@@@@---------------------------------------------------SearchValuesBenchmark.WithSearchValuesInvalidPathChars: DefaultJob [PathCount=1000]Runtime = .NET 9.0.1 (9.0.124.61010), X64 RyuJIT AVX2; GC = Concurrent WorkstationMean = 113.120 us, StdErr = 0.163 us (0.14%), N = 14, StdDev = 0.610 usMin = 111.693 us, Q1 = 112.896 us, Median = 113.052 us, Q3 = 113.307 us, Max = 114.154 usIQR = 0.411 us, LowerFence = 112.280 us, UpperFence = 113.923 usConfidenceInterval = [112.432 us; 113.808 us] (CI 99.9%), Margin = 0.688 us (0.61% of Mean)Skewness = -0.26, Kurtosis = 3.32, MValue = 2-------------------- Histogram --------------------[111.360 us ; 114.186 us) | @@@@@@@@@@@@@@---------------------------------------------------SearchValuesBenchmark.WithIndexOfAnyInvalidPathChars: DefaultJob [PathCount=1000]Runtime = .NET 9.0.1 (9.0.124.61010), X64 RyuJIT AVX2; GC = Concurrent WorkstationMean = 836.442 us, StdErr = 2.263 us (0.27%), N = 15, StdDev = 8.764 usMin = 824.009 us, Q1 = 830.904 us, Median = 836.347 us, Q3 = 841.614 us, Max = 853.035 usIQR = 10.710 us, LowerFence = 814.839 us, UpperFence = 857.678 usConfidenceInterval = [827.073 us; 845.812 us] (CI 99.9%), Margin = 9.369 us (1.12% of Mean)Skewness = 0.24, Kurtosis = 1.94, MValue = 2-------------------- Histogram --------------------[819.345 us ; 857.699 us) | @@@@@@@@@@@@@@@---------------------------------------------------// * Summary *BenchmarkDotNet v0.14.0, Windows 11 (10.0.22631.4602/23H2/2023Update/SunValley3)AMD Ryzen Threadripper 3960X, 1 CPU, 48 logical and 24 physical cores.NET SDK 9.0.102  [Host]     : .NET 9.0.1 (9.0.124.61010), X64 RyuJIT AVX2  DefaultJob : .NET 9.0.1 (9.0.124.61010), X64 RyuJIT AVX2| Method                               | PathCount | Mean         | Error      | StdDev     | Ratio | RatioSD | Rank | Gen0      | Allocated | Alloc Ratio ||------------------------------------- |---------- |-------------:|-----------:|-----------:|------:|--------:|-----:|----------:|----------:|------------:|| WithSearchValuesInvalidFileNameChars | 10        |    33.130 us |  0.6500 us |  0.6080 us |  0.66 |    0.02 |    1 |    9.8267 |   82640 B |        1.00 || WithIndexOfAnyInvalidFileNameChars   | 10        |    49.970 us |  0.9785 us |  0.9153 us |  1.00 |    0.03 |    2 |    9.8267 |   82640 B |        1.00 ||                                      |           |              |            |            |       |         |      |           |           |             || WithSearchValuesInvalidFileNameChars | 100       |   300.889 us |  5.8964 us |  6.5538 us |  0.64 |    0.02 |    1 |  101.0742 |  845840 B |        1.00 || WithIndexOfAnyInvalidFileNameChars   | 100       |   470.042 us |  7.0572 us |  6.6013 us |  1.00 |    0.02 |    2 |  101.0742 |  845840 B |        1.00 ||                                      |           |              |            |            |       |         |      |           |           |             || WithSearchValuesInvalidFileNameChars | 1000      | 3,245.339 us | 63.5488 us | 75.6503 us |  0.63 |    0.02 |    1 | 1011.7188 | 8477882 B |        1.00 || WithIndexOfAnyInvalidFileNameChars   | 1000      | 5,182.520 us | 85.1087 us | 79.6108 us |  1.00 |    0.02 |    2 | 1007.8125 | 8477883 B |        1.00 ||                                      |           |              |            |            |       |         |      |           |           |             || WithSearchValuesInvalidPathChars     | 10        |     1.122 us |  0.0098 us |  0.0092 us |  0.14 |    0.00 |    1 |         - |         - |          NA || WithIndexOfAnyInvalidPathChars       | 10        |     8.156 us |  0.0900 us |  0.0842 us |  1.00 |    0.01 |    2 |         - |         - |          NA ||                                      |           |              |            |            |       |         |      |           |           |             || WithSearchValuesInvalidPathChars     | 100       |    10.059 us |  0.1229 us |  0.1149 us |  0.12 |    0.00 |    1 |         - |         - |          NA || WithIndexOfAnyInvalidPathChars       | 100       |    80.813 us |  0.9032 us |  0.8449 us |  1.00 |    0.01 |    2 |         - |         - |          NA ||                                      |           |              |            |            |       |         |      |           |           |             || WithSearchValuesInvalidPathChars     | 1000      |   113.120 us |  0.6879 us |  0.6098 us |  0.14 |    0.00 |    1 |         - |         - |          NA || WithIndexOfAnyInvalidPathChars       | 1000      |   836.442 us |  9.3694 us |  8.7641 us |  1.00 |    0.01 |    2 |         - |         - |          NA |

PR Context

PR Checklist

Hrxn reacted with thumbs up emojiHrxn reacted with hooray emoji
@ArmaanMcleodArmaanMcleod changed the titleUpdateIndexOfAny calls with invalid path/filename to `SearchValues<char> for more efficient char searchingUpdateIndexOfAny calls with invalid path/filename toSearchValues<char> for more efficient char searchingJan 29, 2025
@iSazonov

This comment was marked as outdated.

@azure-pipelinesAzure Pipelines

This comment was marked as outdated.

@iSazonoviSazonov added the CL-CodeCleanupIndicates that a PR should be marked as a Code Cleanup change in the Change Log labelJan 29, 2025
@iSazonov
Copy link
Collaborator

@ArmaanMcleod If you share perf test results please add more information. You could put your test code under details.
< details>< summary>Details< /summary>
< p>

< /p>
< /details>

@ArmaanMcleod
Copy link
ContributorAuthor

@ArmaanMcleod If you share perf test results please add more information. You could put your test code under details. < details>< summary>Details< /summary> < p>

< /p> < /details>

@iSazonov Please see updated description on the issue. It was a basic bench mark but it seems like substantial perf win from just a small sample.

@iSazonov

This comment was marked as outdated.

@azure-pipelinesAzure Pipelines

This comment was marked as outdated.

@iSazonov
Copy link
Collaborator

@ArmaanMcleod Thanks for benchmark! I updated your OP to hide it under details (you can type slash/ in the editor to get help for Github slash commands)

ArmaanMcleod reacted with hooray emoji

@ArmaanMcleod
Copy link
ContributorAuthor

ArmaanMcleod commentedJan 30, 2025•
edited
Loading

@ArmaanMcleod Thanks for benchmark! I updated your OP to hide it under details (you can type slash/ in the editor to get help for Github slash commands)

Ah thanks@iSazonov . I will do that next time. Maybe we can add a section to pull request template if benchmarking results required?

Might make it easier for authors to provide this for perf improvement PRs.

@iSazonov
Copy link
Collaborator

iSazonov commentedJan 30, 2025•
edited by unfurl-linksbot
Loading

Maybe we can add a section to pull request template if benchmarking results required?

Unfortunately, there is no constant work on performance. So there's not much point in adding this. Although if you want, you can do a PR and add the optional section in the template (with details and link(s) onhttps://github.com/PowerShell/PowerShell/tree/master/test/perf/benchmarks).

GitHub
PowerShell for every system! Contribute to PowerShell/PowerShell development by creating an account on GitHub.
ArmaanMcleod reacted with thumbs up emoji


foreach (string segment in pathWithoutDriveRoot.Split(Path.DirectorySeparatorChar))
{
if (segment.IndexOfAny(invalidFileChars) != -1)
if (PathUtils.ContainsInvalidFileNameChars(segment))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I see we never check full path so we check more short names. Could you please update perf test with loop like this (split path)?

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

@iSazonov Yep good point. I have updated bench mark in description. I've grouped by category filename/path and params (10, 100, 1000) paths so we can bench mark small & large number of paths.

It seems pretty consistent with SearchValues outperforming IndexOfAny with char array.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks! I see.

You could use[Benchmark(Baseline = true)] so that we see Ratio column in results.

ArmaanMcleod reacted with thumbs up emoji
Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

@iSazonov Very nice, I think I should save this benchmark somewhere 😄.

I've updated results. Let me know if anything else is needed, I think this benchmark is quite comprehensive.

iSazonov reacted with thumbs up emoji
@iSazonoviSazonov self-assigned thisJan 30, 2025
@iSazonoviSazonov merged commit2a1d17e intoPowerShell:masterJan 30, 2025
34 of 36 checks passed
@microsoft-github-policy-serviceMicrosoft GitHub Policy Service
Copy link
Contributor

microsoft-github-policy-servicebot commentedJan 30, 2025•
edited by unfurl-linksbot
Loading

📣 Hey@ArmaanMcleod, how did we do? We would love to hear your feedback with the link below! 🗣️

🔗https://aka.ms/PSRepoFeedback

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@iSazonoviSazonoviSazonov approved these changes

Assignees

@iSazonoviSazonov

Labels
CL-CodeCleanupIndicates that a PR should be marked as a Code Cleanup change in the Change Log
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

2 participants
@ArmaanMcleod@iSazonov
[8]ページ先頭
©2009-2025 Movatter.jp