Optimize ReadOnlyTernaryTree for System.Private.Xml

[kronic]

No description provided.

[ghost]

Tagging subscribers to this area: @dotnet/area-system-xml
See info in area-owners.md if you want to be subscribed.

Issue Details

---

null

Author:	kronic
Assignees:	-
Labels:	area-System.Xml
Milestone:	-

[danmoseley]

Thanks @kronic I'll let area owner review this. One thing that would really help reviewers in future would be to keep formatting and functional changes separate. Either a separate PR (if large) or at least a separate commit 😄

[kronic]

@danmoseley I will consider in the future

[eiriktsarpalis]

Hi @kronic, we would need a bit more information on this PR. Would it be possible to 1) provide a synopsis of the performance optimizations you are attempting to make and 2) share benchmarks demonstrating the improvements? Thanks.

[kronic]

@eiriktsarpalis

1. Rather it targets the build size
2. Benchmarks

before System.Private.Xml.dll size 3 087 360 byte

Method	Mean	Error	StdDev	Median	Min	Max	Gen 0	Gen 1	Allocated
WriteStartElement	451.1 ns	9.52 ns	10.58 ns	453.1 ns	429.5 ns	466.2 ns	0.8927	0.0238	7 KB
WriteStartElementIdent	473.2 ns	9.32 ns	8.72 ns	474.9 ns	458.1 ns	488.2 ns	0.8960	0.0243	7 KB

after System.Private.Xml.dll size 3 086 848 byte

Method	Mean	Error	StdDev	Median	Min	Max	Gen 0	Gen 1	Allocated
WriteStartElement	437.8 ns	7.44 ns	7.30 ns	438.6 ns	424.5 ns	454.8 ns	0.8929	0.0237	7 KB
WriteStartElementIdent	465.4 ns	8.97 ns	9.60 ns	465.9 ns	453.4 ns	485.1 ns	0.8951	0.0241	7 KB

#region

using System.IO;
using System.Xml;
using BenchmarkDotNet.Attributes;

#endregion

namespace MicroBenchmarks.libraries.System.Xml.Linq
{
	[BenchmarkCategory(Categories.Libraries)]
	public class Pref_XmlHTml
	{
		private XmlWriterSettings _xmlWriterSettings;
		private XmlWriterSettings _xmlWriterSettings1;
		[GlobalSetup]
		public void Setup()
		{
			_xmlWriterSettings = new XmlWriterSettings
			{
				Indent = false
			};

			_xmlWriterSettings1 = new XmlWriterSettings
			{
				Indent = true
			};

			typeof(XmlWriterSettings).GetProperty(nameof(XmlWriterSettings.OutputMethod))!.SetValue
				(_xmlWriterSettings, XmlOutputMethod.Html);

			typeof(XmlWriterSettings).GetProperty(nameof(XmlWriterSettings.OutputMethod))!.SetValue
				(_xmlWriterSettings1, XmlOutputMethod.Html);
		}
		[Benchmark]
		public void WriteStartElement()
		{
			using var xmlWriter = XmlWriter.Create(Stream.Null, _xmlWriterSettings);
			xmlWriter.WriteStartElement(string.Empty, "WriteStartElement", string.Empty);
		}
		[Benchmark]
		public void WriteStartElementIdent()
		{
			using var xmlWriter = XmlWriter.Create(Stream.Null, _xmlWriterSettings1);
			xmlWriter.WriteStartElement(string.Empty, "WriteStartElementIdent", string.Empty);
		}
	}
}

[krwq]

LGTM thank you! Will only kick off the outerloop tests to double check no regressions

[krwq]

/azp list

[krwq]

/azp run runtime-libraries-coreclr outerloop-windows

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[krwq]

I'll close/reopen PR since something broke with the CI checks

[krwq]

/azp run runtime-libraries-coreclr outerloop-windows

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[jeffhandley]

I've marked this PR as "needs more info" for now. We'd like to better understand the risk/reward for the series of XML changes. See #61773 (comment). Thanks!

[kronic]

I gave my answer Comment

[krwq]

@jeffhandley this particular PR looks like a good optimization to me. In general as we've mentioned in the other PRs: please create an uber issue with motivation and share the approach you're taking so that we can also give you pointers how to achieve performance goals you have and also bucketize types of changes you're making so that any mistakes are easier to catch and fix

[krwq]

/azp run

[azure-pipelines]

You have several pipelines (over 10) configured to build pull requests in this repository. Specify which pipelines you would like to run by using /azp run [pipelines] command. You can specify multiple pipelines using a comma separated list.

[krwq]

/azp run runtime-libraries-coreclr outerloop-windows

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[krwq]

test failures are unrelated