Runtime: Improve XML testing and fuzzing.

[jlennox]

This is an offshoot from issue dotnet/runtime#28217 and PR dotnet/corefx#34116

• [x] Consider bringing *.cxx code generation templates up to date: (per https://github.com/dotnet/corefx/pull/34116#issuecomment-449081565, PR dotnet/corefx#34224)
  
  
  
  • src/System.Private.Xml/src/System/Xml/Core/HtmlRawTextWriterGenerator.cxx
  
  
  • src/System.Private.Xml/src/System/Xml/Core/TextRawTextWriterGenerator.cxx
  
  
  • src/System.Private.Xml/src/System/Xml/Core/XmlRawTextWriterGenerator.cxx
  
  
• [x] Consider rewriting to use programmatic abstractions instead of precompiler code generation. (PR dotnet/corefx#34280)
• [ ] Break code into means that allows better access from inside of tests. (per https://github.com/dotnet/corefx/pull/34116#issuecomment-449081565)
• [ ] Improve test coverage https://ci.dot.net/job/dotnet_corefx/job/master/job/code_coverage_windows/Code_Coverage_Report/ (per https://github.com/dotnet/corefx/pull/34116#issuecomment-447678538)
• [ ] Introduce mechanism to add testing with guarded memory to detect OoB reads. (per https://github.com/dotnet/corefx/pull/34116#issuecomment-449084357)

[jlennox]

@krwq My immediate gut reaction to the .cxx templates is they're a novel solution to a non-novel problem, a problem directly addressed by OO programming + generics. I assume a contributing factor to the use of the preprocessor is performance which evolves as the .net language, runtime, and compiler evolve. I do not know what the considerations were at the time this was originally written. From what I've seen the corefx team makes decisions guided by xunit benchmark results.

I've experimented some with using a generic base class and found some of the code does not translate as easily as I was originally thinking.

buffer[bufPos++] = '[';
RawText(subset);
buffer[bufPos++] = ']';

For example, can't be done as (T)']' and (T)(object)']' would box/destroy performance. Here's the profile results of what pushing the casting through a method would perform like. Release build on dotnet 2.1 running on Windows.

[Benchmark(InnerIterationCount = 100000)]
public unsafe void ProfileDirectCasting()
{
    var bufferArray = new byte[32];
    fixed (byte* buffer = bufferArray)
    foreach (var iteration in Benchmark.Iterations)
    {
        using (iteration.StartMeasurement())
        for (var i = 0; i < Benchmark.InnerIterationCount; i++)
        {
            var bufPos = 0;
            buffer[bufPos++] = (byte)'"';
            buffer[bufPos++] = (byte)'"';
            buffer[bufPos++] = (byte)' ';
            buffer[bufPos++] = (byte)'[';
            buffer[bufPos++] = (byte)']';
            buffer[bufPos++] = (byte)'>';
            buffer[bufPos++] = (byte)'"';
            buffer[bufPos++] = (byte)'"';
            buffer[bufPos++] = (byte)' ';
            buffer[bufPos++] = (byte)'[';
            buffer[bufPos++] = (byte)']';
            buffer[bufPos++] = (byte)'>';
        }
    }
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
private byte ToBufferType(char ch) => (byte)ch;

[Benchmark(InnerIterationCount = 100000)]
public unsafe void ProfileIndirectCasting()
{
    var bufferArray = new byte[32];
    fixed (byte* buffer = bufferArray)
    foreach (var iteration in Benchmark.Iterations)
    {
        using (iteration.StartMeasurement())
        for (var i = 0; i < Benchmark.InnerIterationCount; i++)
        {
            var bufPos = 0;
            buffer[bufPos++] = ToBufferType('"');
            buffer[bufPos++] = ToBufferType('"');
            buffer[bufPos++] = ToBufferType(' ');
            buffer[bufPos++] = ToBufferType('[');
            buffer[bufPos++] = ToBufferType(']');
            buffer[bufPos++] = ToBufferType('>');
            buffer[bufPos++] = ToBufferType('"');
            buffer[bufPos++] = ToBufferType('"');
            buffer[bufPos++] = ToBufferType(' ');
            buffer[bufPos++] = ToBufferType('[');
            buffer[bufPos++] = ToBufferType(']');
            buffer[bufPos++] = ToBufferType('>');
        }
    }
}

                       | Metric                                        | Unit  | Iterations |    Average |    STDEV.S |        Min |        Max
---------------------- |:--------------------------------------------- |:-----:|:----------:| ----------:| ----------:| ----------:| ----------:
ProfileDirectCasting   | Duration                                      | msec  |    100     |      0.354 |      0.080 |      0.286 |      0.602
ProfileDirectCasting   | Branch Mispredictions                         | count |    100     |    204.800 |    897.200 |      0.000 |   4096.000
ProfileDirectCasting   | Cache Misses                                  | count |    100     |     81.920 |    576.329 |      0.000 |   4096.000
ProfileDirectCasting   | Instructions Retired                          | count |    100     | 1.930E+006 | 256432.400 | 1.000E+006 | 2.000E+006
ProfileDirectCasting   | Allocation Size on Benchmark Execution Thread | bytes |    100     |      0.000 |      0.000 |      0.000 |      0.000
ProfileIndirectCasting | Duration                                      | msec  |    100     |      0.360 |      0.089 |      0.285 |      0.595
ProfileIndirectCasting | Branch Mispredictions                         | count |    100     |    327.680 |   1116.815 |      0.000 |   4096.000
ProfileIndirectCasting | Cache Misses                                  | count |    100     |     40.960 |    409.600 |      0.000 |   4096.000
ProfileIndirectCasting | Instructions Retired                          | count |    100     | 1.910E+006 | 287623.491 | 1.000E+006 | 2.000E+006
ProfileIndirectCasting | Allocation Size on Benchmark Execution Thread | bytes |    100     |      0.000 |      0.000 |      0.000 |      0.000

I'd consider the performance hit to be minimal and the potential to make this code far more pleasant to maintain a big bonus, but this is a micro benchmark and not a real world scenario. Thoughts?

[krwq]

@jlennox this was written probably around .NET 1 timeframe :smile: If you're able to get rid of that let's do it :smile: (if there is any perf hit let's not do it though, T4 template should get rid of any hits though and they are much easier to maintain as well)

[jlennox]

@krwq One aspect I find surprising about these unit tests is they're named like EOF_Handling_20, EOF_Handling_21, etc. I assume this also predates written (well, git) history, but do you suspect these were originally generated code? Is me cleaning up their names acceptable to you?

[danmosemsft]

Sure feel free to clean up the names, maybe segregate renames into their own commit?