Reduce allocations

[kostya9]

Noticed some low-hanging fruit when working on parsing CLDR.

• Use StringBuilderPool via Microsoft.Extensions.ObjectPool
• Add some net5.0+ specific optimizations
• Do not go through with unescaping if there is nothing to unescape
• Store Literal InnerText as string

Method	Perf Improvement	Memory Improvement
FormatWithCache	19%	54%
FormatWithCacheEscaped	8%	46%
FormatWithoutCache	15%	54%
FormatWithoutCacheEscaped	8%	52%

Before

Method	Mean	Error	StdDev	Gen 0	Gen 1	Gen 2	Allocated
FormatWithCache	3.956 us	0.0553 us	0.0517 us	1.1444	-	-	3.52 KB
FormatWithCacheEscaped	3.127 us	0.0619 us	0.0662 us	0.9308	-	-	2.85 KB
FormatWithoutCache	7.657 us	0.1434 us	0.1535 us	2.1210	-	-	6.52 KB
FormatWithoutCacheEscaped	5.133 us	0.0398 us	0.0372 us	1.3809	-	-	4.25 KB

After

Method	Mean	Error	StdDev	Gen 0	Gen 1	Gen 2	Allocated
FormatWithCache	3.196 us	0.0185 us	0.0173 us	0.5226	-	-	1.6 KB
FormatWithCacheEscaped	2.883 us	0.0132 us	0.0123 us	0.4959	-	-	1.52 KB
FormatWithoutCache	6.473 us	0.0375 us	0.0351 us	0.9537	-	-	2.95 KB
FormatWithoutCacheEscaped	4.710 us	0.0182 us	0.0161 us	0.6561	-	-	2.02 KB

Benchmark code

    class Program
    {
        static void Main(string[] args)
        {
            BenchmarkRunner.Run<MessageFormatBenchmark>();
        }
    }

    [MemoryDiagnoser]
    [HtmlExporter]
    [CsvExporter]
    public class MessageFormatBenchmark
    {
        private readonly Dictionary<string, object> _params;
        private readonly string _text;
        private readonly string _textEscaped;
        private readonly MessageFormatter _cacheFormatter;
        private readonly MessageFormatter _noCacheFormatter;

        public MessageFormatBenchmark()
        {
            _textEscaped =
                "These '{'count'}' and thoses '{count}' ain''t not escaped, which makes a total of {count, plural, one {a single pair} other {'#'# (=#) pairs}} of escaped braces.";
            _text =
                @"You have {count, plural, 
                            zero {no notifications}
                            one {just one notification}
                            =42 {a universal amount of notifications}
                            other {# notifications}
                      }. Have a nice day!"; ;
            _params = new Dictionary<string, object> { { "count", 2 } };
            _cacheFormatter = new MessageFormatter(true, "ru");
            _noCacheFormatter = new MessageFormatter(false, "ru");
        }

        [GlobalSetup]
        public void SetUp()
        {
            _cacheFormatter.FormatMessage(_text, _params);
            _cacheFormatter.FormatMessage(_textEscaped, _params);
        }

        [Benchmark]
        public string FormatWithCache()
        {
            return _cacheFormatter.FormatMessage(_text, _params);
        }

        [Benchmark]
        public string FormatWithCacheEscaped()
        {
            return _cacheFormatter.FormatMessage(_textEscaped, _params);
        }

        [Benchmark]
        public string FormatWithoutCache()
        {
            return _noCacheFormatter.FormatMessage(_text, _params);
        }

        [Benchmark]
        public string FormatWithoutCacheEscaped()
        {
            return _noCacheFormatter.FormatMessage(_textEscaped, _params);
        }
    }

[jeffijoe]

This is awesome!

While you are in there, would you mind checking these out? If you look at the PR's Files tab, at the bottom these are shown:

[jeffijoe]

I wonder, is there a way to use Span for perf here?

[kostya9]

Here, we get the pluralized value from the cached strings, so I think we can't

[kostya9]

I think we can try using Span APIs more via changing for index StringBuilder loops to GetChunks in NET5.0, but that's a sizeable piece of work in itself )

[jeffijoe]

Ah, so you think GetChunks is faster than an index accessor?

[kostya9]

Yeah, index accessor basically does a lot of duplicating work when iterating through the whole StringBuilder (finds the chunk where the idx is each time)
See here https://github.com/dotnet/runtime/blob/main/src/libraries/System.Private.CoreLib/src/System/Text/StringBuilder.cs#L497

[jeffijoe]

Ah nice find! If we can run the tests against .NET 5 as well as the one that does not support it, I would not be opposed to having 2 different implementations of the most critical paths. Alternatively, we can just bump support to the lowest target that supports Span (even in its less optimal form which should still be more performant)

[kostya9]

The GetChunks API is available starting from netcoreapp3.0
https://docs.microsoft.com/en-us/dotnet/api/system.text.stringbuilder.getchunks?view=net-5.0

I prefer multitargeting for now, because netstandard2.1 does not have the APIs required, and the only next version after netstandard2.0 (that supports both Mono and CoreCLR) with the APIs is net5.0

[jeffijoe]

🔥🔥🔥🔥🔥🔥🔥🔥

[jeffijoe]

I wonder if this is as performant as the Roslyn internal pool 🤔

[kostya9]

Fixed the warning, analyzers are happy now

[jeffijoe]

Excellent work @kostya9 !! 🚀