Reuse StringBuilders in EventArgsFormatting

[rainersigwald]

While I was editing in this area I noticed that the calling pattern around StringBuilders in FormatEventMessage looked allocatey.

Instead of creating two throwaway StringBuilders to format a single message,

1. Make a guess at the initial size based on the maximum format string,
2. Use a StringBuilder from StringBuilderCache, and
3. Reuse the builder between "construct format string" and "get final message"

[benvillalobos]

LGTM. Out of curiosity I grepped for all uses of new StringBuilder and...there are a lot. I doubt most of them are worth converting but one of them may catch someone's eye.

C:\src\git\msbuild>rg -Fi "new StringBuilder"
src\Utilities\ToolLocationHelper.cs
2299:            StringBuilder displayNameBuilder = new StringBuilder();

src\Utilities\TrackedDependencies\DependencyTableCache.cs
138:                StringBuilder normalizedTlogFilename = new StringBuilder();
174:                StringBuilder normalizedTlogPath = new StringBuilder(i + normalizedTlogFilename.Length);

src\Utilities\ToolTask.cs
1580:            StringBuilder sb = new StringBuilder(input);

src\Tasks.UnitTests\ResourceHandling\GenerateResource_Tests.cs
3415:                var sb = new StringBuilder();
3617:            StringBuilder txt = new StringBuilder();
3659:            StringBuilder resgenFileContents = new StringBuilder();

src\Tasks.UnitTests\ResourceHandling\ResGen_Tests.cs
238:            StringBuilder referencePathBuilder = new StringBuilder();

src\Tasks.UnitTests\AppConfig_Tests.cs
212:            StringBuilder b = new StringBuilder();

src\Utilities\CommandLineBuilder.cs
102:        protected StringBuilder CommandLine { get; } = new StringBuilder();

src\Utilities.UnitTests\MockEngine.cs
32:        private StringBuilder _log = new StringBuilder();
85:            set => _log = new StringBuilder(value);

src\StringTools.UnitTests\WeakStringCache_Tests.cs
47:            string testString = new StringBuilder(strPart1).Append(strPart2).ToString();
59:            string testStringCopy = new StringBuilder(strPart1).Append(strPart2).ToString();

src\StringTools\WeakStringCacheInterner.cs
134:            StringBuilder result = new StringBuilder(1024);

src\Tasks\NativeMethods.cs
1456:                StringBuilder sDisplayName = new StringBuilder(ilen);
1487:                                .Aggregate(new StringBuilder(), (builder, v) => builder.Append(v.ToString("x2")))),

src\Tasks\RoslynCodeTaskFactory\RoslynCodeTaskFactory.cs
248:                using (StringWriter writer = new StringWriter(new StringBuilder(), CultureInfo.CurrentCulture))

src\Tasks\XamlTaskFactory\XamlTaskFactory.cs
170:                var errorList = new StringBuilder();

src\Tasks\XamlTaskFactory\XamlDataDrivenToolTask.cs
391:            var switchValue = new StringBuilder(baseSwitch);

src\Tasks\XamlTaskFactory\TaskParser.cs
98:                var parseErrors = new StringBuilder();

src\Tasks\XamlTaskFactory\CommandLineGenerator.cs
560:                    StringBuilder val = new StringBuilder();
586:                StringBuilder val = new StringBuilder();

src\Tasks\WriteCodeFragment.cs
268:            var generatedCode = new StringBuilder();

src\Tasks\RedistList.cs
338:            var keyBuilder = assemblyTables.Length > 0 ? new StringBuilder(assemblyTables[0].Descriptor) : new StringBuilder();
518:            var keyBuilder = whiteListAssemblyTableInfo.Length > 0 ? new StringBuilder(whiteListAssemblyTableInfo[0].Descriptor) : new StringBuilder();

src\Tasks\ParserState.cs
101:            var fullClass = new StringBuilder(1024);

src\Tasks\LockCheck.cs
214:            var key = new StringBuilder(new string('\0', CCH_RM_SESSION_KEY + 1));

src\Tasks\ManifestUtil\Util.cs
65:            StringBuilder s = new StringBuilder(a.Length);
75:            StringBuilder s = new StringBuilder(a.Length);
101:            StringBuilder sb = new StringBuilder(value);

src\Tasks\ManifestUtil\SecurityUtil.cs
791:            var commandLine = new StringBuilder();

src\Tasks\ManifestUtil\FileReference.cs
261:            var sb = new StringBuilder();

src\Tasks\GetAssemblyIdentity.cs
48:            var s = new StringBuilder(a.Length * 2);

src\Tasks\ManifestUtil\AssemblyIdentity.cs
431:            var sb = new StringBuilder(_name);

src\Tasks\GenerateResource.cs
2046:                StringBuilder sb = new StringBuilder(text.Length);
3561:                StringBuilder name = new StringBuilder(255);
3562:                StringBuilder value = new StringBuilder(2048);

src\Shared\XamlUtilities.cs
31:            StringBuilder propertyId = new StringBuilder();

src\Shared\UnitTests\ObjectModelHelpers.cs
354:            StringBuilder expectedItemSpecs = new StringBuilder();
366:            StringBuilder actualItemSpecs = new StringBuilder();
623:            StringBuilder sb = new StringBuilder(xml.Length);
1561:            var sb = new StringBuilder(64);

src\Shared\UnitTests\MockLogger.cs
34:        private StringBuilder _fullLog = new StringBuilder();
213:                _fullLog = new StringBuilder();

src\Shared\UnitTests\MockEngine.cs
39:        private readonly StringBuilder _log = new StringBuilder();

src\Shared\UnitTests\EngineTestEnvironment.cs
173:            var sb = new StringBuilder();
181:            var sb = new StringBuilder();

src\Shared\TaskLoggingHelper.cs
941:                var builder = new StringBuilder(200);

src\Shared\StringExtensions.cs
35:            var builder = new StringBuilder(aString.Length - oldValue.Length + newValue.Length);

src\Shared\QuotingUtilities.cs
76:            string separators = new StringBuilder().Append(separator).ToString();
80:            StringBuilder splitString = new StringBuilder();
183:            StringBuilder unquotedString = new StringBuilder();

src\Tasks\BootstrapperUtil\BootstrapperBuilder.cs
318:                            StringBuilder missingProductCodes = new StringBuilder();
340:                        var missingProductCodes = new StringBuilder();
514:                StringBuilder productsOrder = new StringBuilder();
734:            var productsInLoop = new StringBuilder();
1628:            var output = new StringBuilder(byteArray.Length);

src\Tasks\FileIO\WriteLinesToFile.cs
63:                StringBuilder buffer = new StringBuilder();

src\Tasks\CreateManifestResourceName.cs
377:            var everettId = new StringBuilder(name.Length);

src\Tasks\DataDriven\DataDrivenToolTask.cs
605:            StringBuilder val = new StringBuilder(GetEffectiveArgumentsValues(toolSwitch));
644:                    StringBuilder val = new StringBuilder(GetEffectiveArgumentsValues(toolSwitch));
670:                StringBuilder val = new StringBuilder(GetEffectiveArgumentsValues(toolSwitch));

src\Shared\LanguageParser\StreamMappedString.cs
376:            StringBuilder result = new StringBuilder(length);

src\Shared\FileUtilities.cs
144:            StringBuilder builder = new StringBuilder();

src\Tasks\ComReference.cs
410:            var buffer = new StringBuilder();

src\Tasks\CommandLineBuilderExtension.cs
167:            StringBuilder quotedText = new StringBuilder();

src\Shared\ExceptionHandling.cs
367:            var builder = new StringBuilder();

src\Shared\FileMatcher.cs
2201:            var sb = new StringBuilder(aString.Length);

src\Tasks\CodeTaskFactory.cs
757:                var codeBuilder = new StringBuilder();

src\Tasks\AxTlbBaseReference.cs
231:            var builder = new StringBuilder(interopDllHeader);

src\Shared\EventArgsFormatting.cs
222:            StringBuilder format = new StringBuilder();
333:            StringBuilder formattedMessage = new StringBuilder();

src\Tasks\AssemblyDependency\ResolveAssemblyReference.cs
1093:                            // If we are logging warnings append it into existing StringBuilder, otherwise build details by new StringBuilder.
1095:                            StringBuilder logDependencies = logWarning ? logConflict.AppendLine() : new StringBuilder();
1139:                        var buffer = new StringBuilder();
1292:            var buffer = new StringBuilder(a.Length * 2);

src\Tasks\AssemblyDependency\GlobalAssemblyCache.cs
376:            StringBuilder gacPath = new StringBuilder(gacPathLength);

src\Tasks\AssemblyDependency\AssemblyInformation.cs
581:                    runtimeVersion = new StringBuilder(bufferLength);

src\Shared\ConversionUtilities.cs
60:            var sb = new StringBuilder();

src\Shared\AssemblyNameExtension.cs
879:            StringBuilder sb = new StringBuilder(displayName);

src\StringTools\SpanBasedStringBuilder.Simple.cs
85:            _builder = new StringBuilder(capacity * 128);

src\StringTools.Benchmark\SpanBasedStringBuilder_Benchmark.cs
23:        private static StringBuilder _pooledStringBuilder = new StringBuilder();

src\MSBuild\XMake.cs
173:            StringBuilder builder = new StringBuilder();

src\MSBuild\PerformanceLogEventListener.cs
139:                    s_builder = new StringBuilder();

src\Deprecated\Engine.UnitTests\ProjectIdLogger.cs
16:        private StringBuilder fullLog = new StringBuilder();

src\Deprecated\Engine.UnitTests\Choose_Tests.cs
31:            StringBuilder sb1 = new StringBuilder();
32:            StringBuilder sb2 = new StringBuilder();

src\Deprecated\Engine.UnitTests\ConsoleLogger_Tests.cs
28:                simulatedConsole = new StringBuilder();
33:                simulatedConsole = new StringBuilder();

src\Deprecated\Engine.UnitTests\Compatibility\Project_Tests.cs
1793:                StringBuilder stringBuilder = new StringBuilder();

src\Framework\TaskParameterEventArgs.cs
69:            var sb = new StringBuilder();

src\Framework\StringBuilderCache.cs
15:**            subsequent calls will return a new StringBuilder.
61:                    // Avoid StringBuilder block fragmentation by getting a new StringBuilder
74:            StringBuilder stringBuilder = new StringBuilder(capacity);

src\Framework\ReuseableStringBuilder.cs
237:                    returned = new StringBuilder(SelectBracketedCapacity(capacity));
252:                    returned = new StringBuilder(newCapacity);
307:                        returningBuilder = new StringBuilder(newCapacity);

src\Framework\NativeMethods.cs
962:                StringBuilder fullPathBuffer = new StringBuilder(length);
1001:                StringBuilder fullPathBuffer = new StringBuilder(length);

src\Build\Graph\ProjectGraph.cs
496:            var sb = new StringBuilder();

src\Build.UnitTests\SolutionFileBuilder.cs
148:            var sb = new StringBuilder();

src\Build\Graph\GraphBuilder.cs
593:            var errorMessage = new StringBuilder(projectsInCycle.Select(p => p.Length).Sum());

src\Build.UnitTests\TargetsFile_Test.cs
1997:            StringBuilder deepRelativePath = new StringBuilder();

src\Build.UnitTests\ProjectCache\ProjectCacheTests.cs
691:            var sb = new StringBuilder();

src\Build.UnitTests\Graph\ResultCacheBasedBuilds_Tests.cs
515:            var sb = new StringBuilder();

src\Build.UnitTests\Evaluation\ProjectStringCache_Tests.cs
287:            StringBuilder builder = new StringBuilder();
321:            StringBuilder builder = new StringBuilder();
363:            StringBuilder builder = new StringBuilder();
400:            StringBuilder builder = new StringBuilder();
435:            StringBuilder builder = new StringBuilder();
469:            StringBuilder builder = new StringBuilder();

src\Build.UnitTests\Evaluation\ProjectSdkImplicitImport_Tests.cs
590:                var result = new StringBuilder(256);

src\Build.UnitTests\Evaluation\ItemEvaluation_Tests.cs
537:            StringBuilder content = new StringBuilder();

src\Build\Logging\BaseConsoleLogger.cs
162:            StringBuilder result = new StringBuilder(
283:            StringBuilder result = new StringBuilder((indentLevel * tabWidth) + formattedString.Length);

src\Build\Logging\BinaryLogger\BuildEventArgsReader.cs
1295:                    stringBuilder = new StringBuilder();

src\Build\Evaluation\Profiler\ProfilerResultPrettyPrinter.cs
37:            var stringBuilder = new StringBuilder();

src\Build\Evaluation\Evaluator.cs
2551:            var sb = new StringBuilder();

src\Build.UnitTests\ConsoleLogger_Tests.cs
53:                _simulatedConsole = new StringBuilder();
58:                _simulatedConsole = new StringBuilder();

src\Build\Construction\Solution\ProjectInSolution.cs
498:            var cleanProjectName = new StringBuilder(projectName);

src\Build\Construction\Solution\SolutionProjectGenerator.cs
232:            var solutionConfigurationContents = new StringBuilder(1024);
550:            var builder = new StringBuilder(name);
1018:                var xml = new StringBuilder();
1425:                var conditionDescribingValidConfigurations = new StringBuilder("(false)");
1446:                StringBuilder referenceItemName = new StringBuilder(GenerateSafePropertyName(project, "References"));
1605:            var referenceGuids = new StringBuilder();
1766:            var condition = new StringBuilder(" ('$(CurrentSolutionConfigurationContents)' != '') and (false");

src\Build.UnitTests\BuildEventArgsSerialization_Tests.cs
615:            StringBuilder sb = new StringBuilder();

src\Build\BackEnd\TaskExecutionHost\TaskExecutionHost.cs
1458:                    StringBuilder joinedOutputs = (outputs.Length == 0) ? new StringBuilder() : null;
1465:                            joinedOutputs ??= new StringBuilder();
1534:                    StringBuilder joinedOutputs = (outputs.Length == 0) ? new StringBuilder() : null;
1541:                            joinedOutputs ??= new StringBuilder();

src\Build\BackEnd\Components\Scheduler\Scheduler.cs
2273:            StringBuilder nodeIndices = new StringBuilder();
2366:            StringBuilder utilitzationPercentages = new StringBuilder();
2390:            StringBuilder stringBuilder = new StringBuilder(64);
2458:            StringBuilder prePadString = new StringBuilder(2 * level);

src\Build.UnitTests\BinaryLogger_Tests.cs
91:            var serialFromBuildText = new StringBuilder();
95:            var parallelFromBuildText = new StringBuilder();
104:            var serialFromPlaybackText = new StringBuilder();
108:            var parallelFromPlaybackText = new StringBuilder();

src\Build.UnitTests\BackEnd\TranslationHelpers.cs
143:            var sb = new StringBuilder();

src\Build.UnitTests\BackEnd\CustomTaskHelper.cs
53:                    StringBuilder builder = new StringBuilder();

src\Framework\BuildEventArgs.cs
291:            var sb = new StringBuilder();

src\Deprecated\Conversion\OldVSProjectFileReader.cs
62:            this.singleLine = new StringBuilder();
253:            StringBuilder result = new StringBuilder();
316:                this.singleLine = new StringBuilder(this.ReplaceSpecialCharacters(lineFromProjectFile));
553:            StringBuilder replacedString = new StringBuilder();

src\Deprecated\Conversion\AdditionalOptionsParser.cs
169:                new StringBuilder(),
178:                new StringBuilder(),
245:            StringBuilder option = new StringBuilder();

src\Deprecated\Engine\Shared\UnitTests\ObjectModelHelpers.cs
196:            StringBuilder expectedItemSpecs = new StringBuilder();
208:            StringBuilder actualItemSpecs = new StringBuilder();

src\Deprecated\Engine\Shared\UnitTests\MockLogger.cs
32:        private StringBuilder fullLog = new StringBuilder();
168:            this.fullLog = new StringBuilder();

src\Deprecated\Engine\Shared\UnitTests\FileUtilities_Tests.cs
288:            StringBuilder sb = new StringBuilder(NativeMethods.MAX_PATH);

src\Deprecated\Engine\Shared\ProjectInSolution.cs
353:            StringBuilder cleanProjectName = new StringBuilder(projectName);

src\Deprecated\Engine\Shared\NativeMethodsShared.cs
56:            StringBuilder pathBuilder = new StringBuilder(MAX_PATH + 1);

src\Deprecated\Engine\Shared\FileUtilities.cs
684:                    StringBuilder sb = new StringBuilder(NativeMethods.MAX_PATH);

src\Deprecated\Engine\Shared\EventArgsFormatting.cs
121:            StringBuilder format = new StringBuilder();
218:            StringBuilder formattedMessage = new StringBuilder();

src\Deprecated\Engine\Shared\FileMatcher.cs
738:            StringBuilder matchFileExpression = new StringBuilder();

src\Deprecated\Engine\Shared\EscapingUtilities.cs
58:            StringBuilder unescapedString = new StringBuilder();
124:            StringBuilder escapedString = new StringBuilder(unescapedString);

src\Deprecated\Engine\Shared\AssemblyNameExtension.cs
488:            StringBuilder sb = new StringBuilder(displayName);

src\Deprecated\Engine\Solution\SolutionWrapperProject.cs
701:            StringBuilder referenceGuids = new StringBuilder();
818:            StringBuilder referenceItemName = new StringBuilder(GenerateSafePropertyName(proj, "References"));
825:            StringBuilder importLibraryItemName = new StringBuilder(GenerateSafePropertyName(proj, "ImportLibraries"));
1279:            StringBuilder condition = new StringBuilder(" ('$(CurrentSolutionConfigurationContents)' != '') and (false");
1374:                StringBuilder conditionDescribingValidConfigurations = new StringBuilder("(false)");
1397:                StringBuilder referenceItemName = new StringBuilder(GenerateSafePropertyName(proj, "References"));
1468:            StringBuilder builder = new StringBuilder(name);
1665:            StringBuilder dependencies = new StringBuilder();
1706:            StringBuilder dependencies = new StringBuilder();
1793:            StringBuilder solutionConfigurationContents = new StringBuilder("<SolutionConfiguration>");

src\Deprecated\Engine\Logging\ParallelLogger\ParallelConsoleLogger.cs
670:                StringBuilder result = new StringBuilder();

src\Deprecated\Engine\Logging\BaseConsoleLogger.cs
184:            StringBuilder result = new StringBuilder(
295:            StringBuilder result = new StringBuilder();

src\Deprecated\Engine\Items\ItemExpander.cs
354:                StringBuilder expandedItemVector = new StringBuilder();

src\Deprecated\Engine\Engine\TaskExecutionState.cs
259:        private static StringBuilder currentDirectoryBuffer = new StringBuilder(270);

src\Deprecated\Engine\Engine\TaskExecutionModule.cs
645:        private static StringBuilder currentDirectoryBuffer = new StringBuilder(270);

src\Deprecated\Engine\Engine\TaskEngine.cs
1009:                    StringBuilder joinedOutputs = new StringBuilder();
1060:                    StringBuilder joinedOutputs = new StringBuilder();

src\Deprecated\Engine\Engine\TargetDependencyAnalyzer.cs
374:                StringBuilder inputsBuilder = new StringBuilder();
391:                StringBuilder outputsBuilder = new StringBuilder();

src\Deprecated\Engine\Engine\Expander.cs
554:                StringBuilder result = new StringBuilder(expression.Length);
596:                    StringBuilder builder = new StringBuilder();
617:                    StringBuilder builder = new StringBuilder();
1587:                StringBuilder argumentBuilder = new StringBuilder(argumentsContent.Length);
1623:                        argumentBuilder = new StringBuilder(argumentsContent.Length);

src\Deprecated\Engine\Engine\BuildRequest.cs
593:                    StringBuilder targetsBuilder = new StringBuilder();

src\Build.OM.UnitTests\Definition\Project_Tests.cs
2839:            var sb = new StringBuilder();

src\Build.OM.UnitTests\Construction\SolutionFile_Tests.cs
1045:            StringBuilder stringBuilder = new StringBuilder();

src\Build.OM.UnitTests\Construction\ProjectRootElement_Tests.cs
585:            StringBuilder builder = new StringBuilder();

src\Build.OM.UnitTests\Construction\ProjectChooseElement_Tests.cs
248:                StringBuilder builder1 = new StringBuilder();
249:                StringBuilder builder2 = new StringBuilder();

src\Build.OM.UnitTests\Construction\ConstructionEditing_Tests.cs
3250:            var sb = new StringBuilder();

[Forgind]

LGTM. Out of curiosity I grepped for all uses of new StringBuilder and...there are a lot. I doubt most of them are worth converting but one of them may catch someone's eye.

/cc: @elachlan if you're interested. I wouldn't bother with tests or code that just runs a couple times, but if you've noticed something that runs a lot and feel motivated to fix it...

[elachlan]

LGTM. Out of curiosity I grepped for all uses of new StringBuilder and...there are a lot. I doubt most of them are worth converting but one of them may catch someone's eye.

/cc: @elachlan if you're interested. I wouldn't bother with tests or code that just runs a couple times, but if you've noticed something that runs a lot and feel motivated to fix it...

Thanks, maybe create an issue and I'll see if I can get around to it. The Analyzers are keeping me pretty busy at the moment.

[rainersigwald]

Talked to @rokonec about this and we think based on the numbers from #2697 (comment) that using the full-strength ReusableStringBuilder here is likely a good idea, so I'm going to do that.

[rainersigwald]

Talked to @rokonec about this and we think based on the numbers from #2697 (comment) that using the full-strength ReusableStringBuilder here is likely a good idea, so I'm going to do that.

Done.