Is this expected to be a perf improvement? Do you have any numbers (for the public Environment APIs) to demonstrate that this is better?

Is this expected to be a perf improvement? Do you have any numbers (for the public Environment APIs) to demonstrate that this is better?

This was expected to be a performance improvement as it avoids the overhead of initialising a Guid from a string, see https://csharp.godbolt.org/z/vxKqKjT4r

However System.Tests.Perf_Environment.GetFolderPath benchmark fails to show significant improvements.

How about code size metrics? Is this a binary size improvement, e.g. for NAOT?

How about code size metrics? Is this a binary size improvement, e.g. for NAOT?

@EgorBo Could you help verify this?

@MihuBot

MihuBot

@MihaZupan These changes are specific to Windows target, is this an option with MihuBot?

Windows target

No, hasn't been wired through for the JIT diffs

1kb is not that much and it is 4kB regression for non-NAOT (16,031,744 -> 16,035,840 - there can be some rounding error in this).

Have you tried to actually step through the code to see where to make this better? I see:

• The public entrypoint calls Enum.IsDefined (potentially two times) that is fairly inefficient. We can rid of the GetFolderPath/GetFolderPathCore split of the worker method and Enum.IsDefined calls on the main path.
  • For SpecialFolderOption, we can check the 3 valid values.
  • For SpecialFolder, we can throw in the default case of the switch statement (avoid Enum.IsDefined completely), or call Enum.IsDefined in the default case of the switch statement only (pay for Enum.IsDefined at runtime in rare cases only)
• The Guid properties are not inlined on Windows. There is a lot of extra code to pass the return buffers, etc. If we care about making this efficient, would it be better to store these as byte arrays? Like, internal static ReadOnlySpan<byte> AdminTools => [0x.., 0x.., ];.

@EgorBot --filter "System.Tests.Perf_Environment.GetFolderPath"

I do not expect that this ReadOnlySpan change is going improve throughput over the previous version. It is code size improvement.

You may see some throughput improvement if you get rid of Enum.IsDefined calls.

@EgorBo There is no correctness issue for big-endian platforms, since we call Guid(ReadOnlySpan).

Using Unsafe.As<byte, Guid>(ref MemoryMarshal.GetReference(folderId)) was considered and rejected.

4kb-5kB improvement for NAOT MichalStrehovsky/rt-sz#159 (comment)