Improve performance of Environment.GetEnvironmentVariable

[janvorli]

This change improves performance of Environment.GetEnvironmentVariable in release
build by about 25%.
To do that, I have heavily refactored the UTF8ToUnicode and UnicodeToUTF8 functions
and replaced malloc in GetEnvironmentVariableW by the recently checked in StackString.

To make sure the UTF8ToUnicode and UnicodeToUTF8 functions work properly, I have
added two PAL tests that extensively test the conversion including hopefully all
corner cases.

[janvorli]

@adityamandaleeka can you take a look please?

[janvorli]

FYI: @ianhays, @stephentoub, @ellismg

[jkotas]

It may be better to take the managed UTF8 encoder/decoder implementation from src\mscorlib\src\System\Text\UTF8Encoding.cs (with minimal changes to make it C++, and otherwise fit here).

From cursory look, src\mscorlib\src\System\Text\UTF8Encoding.cs implementation is faster than what you got; and it is also pretty well tested.

[janvorli]

@jkotas I did some testing of the managed decoder vs my one that I've very slightly enhanced based on the idea of reading / writing multiple characters at once.
First, I've used a UTF-8 document with mostly ascii characters, but also containing some characters encoded using various lengths. This document was 22781 bytes long. I've tested also the unicode to utf-8 by encoding the result of the utf-8 to unicode back to utf8.
The second document was a czech book 1,415,650 bytes long with a mix of 1 and 2 byte encoded characters. Again, I've tested the unicode to utf-8 by encoding the result of the utf-8 to unicode back to utf8.
Here are the results (both in release builds):
Small document:

Format	Managed	Native
utf8 -> unicode	177us	69 us
unicode -> utf8	162us	48 us

Large document:

Format	Managed	Native
utf8 -> unicode	8.02ms	8.5ms
unicode -> utf8	7.05ms	6.3ms

Based on these results, I'd prefer keeping my refactored version. Especially based on the fact that we use the functions to translate filenames and environment variable names / contents where I'd expect mostly ascii characters with some two byte encoded ones.

Does it make sense?

[jkotas]

These numbers do not make sense. Can you share code for your benchmark? There is number of factors that can explain them - code quality diff between JIT and C/C++ compiler, ... - hard to tell without seeing the actual benchmark.

[janvorli]

Here is the managed code:

    class Program
    {
        static void Main(string[] args)
        {
            if (args.Length != 2) 
            {
                Console.WriteLine("Usage: [unicode|utf8] Utf8PerfTestManaged file");
                return;
            }

            bool sourceIsUtf8 = args[0] == "utf8";

            using (FileStream fs = File.OpenRead(args[1])) 
            {
                byte[] source = new byte[fs.Length];
                fs.Read(source, 0, (int)fs.Length);
                byte[] destination = null;
                var sw = new Stopwatch();
                sw.Restart();
                if (sourceIsUtf8) {
                    destination = Encoding.Convert(Encoding.UTF8, Encoding.Unicode, source);
                }
                else {
                    destination = Encoding.Convert(Encoding.Unicode, Encoding.UTF8, source);
                }
                Console.WriteLine(sw.Elapsed);

                using (FileStream fs2 = File.OpenWrite(@"D:\test\Utf8PerfTestManaged\targetmanaged.txt")) 
                {
                    fs2.Write(destination, 0, destination.Length);
                }
            }
        }
    }

[jkotas]

Encoding.Convert(Encoding.UTF8, Encoding.Unicode, source);

This is low performance convenience method - it will run the UTF8 encoder two times, and allocate big two arrays on the GC heap along the way. I doubt your unmanaged equivalent does that...

You should be benchmarking the high performance entrypoints like int UTF8Encoding.GetBytes(char* chars, int charCount, byte* bytes, int byteCount) that match your unmanaged equivalent.

[jkotas]

Maintaining two structurally different optimized mutations of the UTF8 encoding/decoding core algorithm does not make sense. The core algorithm used in UTF8Encoding.cs has been bug-fixed and fine-tuned by number of people over the years, and it is replicated in number of places in different contexts (across Microsoft codebases).

I want us to:

• Use the same structure of the algorithm in the PAL
• If there is performance tweak that you would like to make to it, I want the same tweak to be done for both unmanaged PAL and managed implementation, and eventually other replicas. The performance of the managed implementation actually matters more than the performance of unmanaged PAL implementation.

[janvorli]

@jkotas ok, I understand your point on unifying the implementation and it makes sense. The goal of my work was to do a simple refactoring to speed it up. But I ended up doing small incremental changes over the days in spare moments and in the end it got to the completely refactored state.

Out of curiosity, not to push my solution, I've changed the managed benchmark to use the low level functions (well, not the ones with the pointers, since they are not public, but the ones that get arrays and just pin them before calling the low level stuff.

Here are the results:

Format	Managed	Native
utf8 -> unicode	76us	69 us
unicode -> utf8	55us	48 us

Large document:

Format	Managed	Native
utf8 -> unicode	5.7ms	8.5ms
unicode -> utf8	4.8ms	6.3ms

The new source is here. It does exactly what the native one does - go through the data twice, once to get the necessary size of the destination buffer and then again to perform the conversion.

    class Program
    {
        static void Main(string[] args)
        {
            if (args.Length != 2) 
            {
                Console.WriteLine("Usage: [unicode|utf8] Utf8PerfTestManaged file");
                return;
            }

            bool sourceIsUtf8 = args[0] == "utf8";

            if (sourceIsUtf8) {
                byte[] source = File.ReadAllBytes(args[1]);
                char[] destination = new char[source.Length * 4];
                var sw = new Stopwatch();
                UTF8Encoding utf8Encoding = new UTF8Encoding();
                for (int i = 0; i < 10; i++) {
                    sw.Restart();
                    int c = utf8Encoding.GetCharCount(source, 0, source.Length);
                    utf8Encoding.GetChars(source, 0, source.Length, destination, 0);
                    Console.WriteLine(sw.Elapsed);
                }
            }
            else
            {
                string source = File.ReadAllText(args[1]);
                byte[] destination = new byte[source.Length * 4];
                var sw = new Stopwatch();
                UTF8Encoding utf8Encoding = new UTF8Encoding();
                for (int i = 0; i < 10; i++) {
                    sw.Restart();
                    int c = utf8Encoding.GetByteCount(source);
                    utf8Encoding.GetBytes(source, 0, source.Length, destination, 0);
                    Console.WriteLine(sw.Elapsed);
                }
            }
        }
    }

[stephentoub]

@janvorli, is this PR still relevant?

[janvorli]

No, it is not, since @wtgodbe has already ported the C# version.