Performance optimization of FileSystem.CreateDirectory on windows

[deeprobin]

⚠️ THIS IS A DRAFT PR - DO NOT MERGE

Closes #61954

Current state of implementation

• Initial analysis
• Code-readability improvements
• Minor perf improvements
• Advanced analysis: Benchmarks, Profiling, ...

[ghost]

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

Issue Details

---

⚠️ THIS IS A DRAFT PR - DO NOT MERGE

Closes #61954

Current state of implementation

• Initial analysis
• Code-readability improvements
• Minor perf improvements
• Advanced analysis: Benchmarks, Profiling, ...

Author:	deeprobin
Assignees:	-
Labels:	area-System.IO
Milestone:	-

[deeprobin]

Benchmark results .NET 6

dotnet run -c Release -f net6.0 --filter System.IO.Tests.Perf_Directory.CreateDirectory

Method	Mean	Error	StdDev	Median	Min	Max	Allocated
CreateDirectory	184.1 us	4.15 us	4.44 us	183.5 us	178.3 us	191.2 us	497 B

Benchmark results .NET 7

dotnet run -c Release -f net7.0 --filter System.IO.Tests.Perf_Directory.CreateDirectory

Method	Mean	Error	StdDev	Median	Min	Max	Allocated
CreateDirectory	212.8 us	26.49 us	30.51 us	208.7 us	173.7 us	280.5 us	497 B

Regression from .NET 6 😦

Benchmarks after this change

dotnet run -c Release -f net7.0 --filter System.IO.Tests.Perf_Directory.CreateDirectory --coreRun E:\external\dotnet\runtime\artifacts\bin\testhost\net7.0-windows-Release-x64\shared\Microsoft.NETCore.App\7.0.0\CoreRun.exe

Method	Mean	Error	StdDev	Median	Min	Max	Allocated
CreateDirectory	512.2 us	18.84 us	20.94 us	504.6 us	489.2 us	569.9 us	1 KB

and my change regresses too ☹️
@danmoseley Do you know why? I have actually reduced the kernel calls and so this should be faster.

[deeprobin]

Profiling result (.NET 6)

const int iterations = 10_000_000;
for (var i = 0; i < iterations; i++)
{
    Directory.CreateDirectory($"Test/{i}");
}

[deeprobin]

I've done some benchmarks:

INITIAL

Method	Mean	Error	StdDev	Median	Min	Max	Allocated
CreateDirectory	224.6 us	15.73 us	18.12 us	222.1 us	201.0 us	263.8 us	497 B

PathExists

Method	Mean	Error	StdDev	Median	Min	Max	Allocated
CreateDirectory	208.9 us	10.43 us	11.59 us	207.2 us	190.9 us	231.1 us	497 B

removalIndex

Method	Mean	Error	StdDev	Median	Min	Max	Allocated
CreateDirectory	213.8 us	12.44 us	14.32 us	205.7 us	196.7 us	237.7 us	497 B

Usage of one ReadOnlySpan

Method	Mean	Error	StdDev	Median	Min	Max	Allocated
CreateDirectory	215.3 us	13.74 us	14.70 us	211.5 us	197.0 us	253.0 us	497 B

Usage of ReadOnlySpan.Slice instead of string.Substring

Method	Mean	Error	StdDev	Median	Min	Max	Allocated
CreateDirectory	213.5 us	11.06 us	12.30 us	212.4 us	195.4 us	240.3 us	617 B

Remove unnessecary path check

Method	Mean	Error	StdDev	Median	Min	Max	Allocated
CreateDirectory	213.8 us	11.36 us	11.67 us	211.7 us	194.8 us	237.0 us	617 B

List preallocation

Method	Mean	Error	StdDev	Median	Min	Max	Allocated
CreateDirectory	199.4 us	4.40 us	4.32 us	198.9 us	192.1 us	210.2 us	529 B

Style changes

Method	Mean	Error	StdDev	Median	Min	Max	Allocated
CreateDirectory	216.6 us	11.87 us	13.68 us	217.5 us	190.7 us	239.1 us	529 B

Style changes and only one Span usage

Method	Mean	Error	StdDev	Median	Min	Max	Allocated
CreateDirectory	205.4 us	6.75 us	6.94 us	202.7 us	198.6 us	226.4 us	529 B

[deeprobin]

Benchmark "System.IO.Tests.Perf_Directory.CreateDirectory" Results on AMD Ryzen 5 3600

[deeprobin]

In my opinion, unfortunately, you can not optimize much more, because quite a lot of performance CreateDirectoryW is lost, which we can not influence.

I am of course open to further suggestions.

[stephentoub]

Thanks for your interest in contributing here, but I don't understand what it is you're actually improving. I see a lot of stylistic changes (which we ask not be made as part of the same PRs making perf / functional changes), but I see little to no changes that would positively impact performance (I see some that could theoretically help in some cases but similarly hurt in others, e.g. preinitializing a list to be able to store 8 elements).

[deeprobin]

Thanks for your interest in contributing here, but I don't understand what it is you're actually improving. I see a lot of stylistic changes (which we ask not be made as part of the same PRs making perf / functional changes), but I see little to no changes that would positively impact performance (I see some that could theoretically help in some cases but similarly hurt in others, e.g. preinitializing a list to be able to store 8 elements).

Yes, the bottleneck seems to be in CreateDirectoryW (see profiling result) and without affecting the current behavior of the previous operations I currently see little possibility to do anything here performance-wise.

I will start the branch again (without style changes) and see if I can do some micro optimization.

One tweak I made is to not duplicate the AttributeInfo retrieval, which made little improvement for me after the benchmarks though ¹.

I would then apply this change. The preinitialization of the List of course only makes sense if we know in advance how many entries we will get.
Do you have an idea how to replace the List? The List already caught my eye at the beginning (allocations ² 😩 / one per path-segment).

Footnotes

1. See https://github.com/dotnet/runtime/pull/62860/commits/6986b2cc7425f6663ba663d3e7f3a1ea8cefb73d ↩

2. I have nothing against allocations, of course, but I think in this case it would be good to reduce them. ↩

[stephentoub]

Thanks for your interest in improving performance. I'm going to close this until there's a demonstrable improvement.

[deeprobin]

Thank you 👍🏼.
There are currently more important things that need to be done. The performance can be looked at again later (or someone else creates a PR / or commits into this).