Add char[] buffer to XmlRawWriter.#75411
Add char[] buffer to XmlRawWriter.#75411TrayanZapryanov wants to merge 11 commits intodotnet:mainfrom
Conversation
Expose TryFormat methods in XmlConvert and use them in XmlRawWriter. Expose TryFormat in XsdDateTime and XsdDuration.
|
Tagging subscribers to this area: @dotnet/area-system-xml Issue DetailsThis PR suggest using small char[] buffer in XmlRawWriter and use it for formatting primitive types in it and then using WriteRaw(char[]) method instead of creating temporary string and using WriteRaw(string). This PR will open possibilities in XmlSerializers to use typed methods of XmlWriter instead of always fallback to WriteString one.
|
src/libraries/System.Private.Xml/src/System/Xml/Core/XmlRawWriter.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.Xml/src/System/Xml/Core/XmlRawWriter.cs
Outdated
Show resolved
Hide resolved
| _primitivesBuffer = new char[_primitivesBuffer.Length * 2]; | ||
| } | ||
|
|
||
| WriteChars(_primitivesBuffer, 0, charsWritten); |
There was a problem hiding this comment.
I saw that there is difference between
and
Which one should be used ?
| { | ||
| // For compatibility with custom writers, XmlWriter writes DateTimeOffset as DateTime. | ||
| // Our internal writers should use the DateTimeOffset-String conversion from XmlConvert. | ||
| WriteString(XmlConvert.ToString(value)); |
There was a problem hiding this comment.
This comment seems not relevant as even now it is using
There was a problem hiding this comment.
Maybe it is connected with base code ?
There was a problem hiding this comment.
@krwq Do you know if somebody is changing this Mode ?
I saw similar code in XmlSerializers and the only way to change is with Reflection and directly manipulating fiedl.
src/libraries/System.Xml.ReaderWriter/ref/System.Xml.ReaderWriter.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.Xml/src/System/Xml/Core/XmlRawWriter.cs
Outdated
Show resolved
Hide resolved
| WriteWithBuffer((char)value, XmlConvert.TryFormat); | ||
| break; | ||
| default: | ||
| //Guid is not supported in XmlUntypedConverter.Untyped and throws exception |
There was a problem hiding this comment.
This limitation is really strange, but we need to adapt XmlValueConverter to allow Guid in it's methods.
There was a problem hiding this comment.
Added proposal for this Add WriteValue(Guid/TimeSpan) in System.Xml.XmlWriter
| return CreateException(index == 0 ? SR.Xml_BadStartNameChar : SR.Xml_BadNameChar, XmlException.BuildCharExceptionArgs(name, index), exceptionType, 0, index + 1); | ||
| } | ||
|
|
||
| public static bool TryFormat(bool value, Span<char> destination, out int charsWritten) |
There was a problem hiding this comment.
I believe this one can do same optimization as in #64782 but with correct casing.
src/libraries/System.Private.Xml/src/System/Xml/Core/XmlRawWriter.cs
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| // Override in order to handle Xml simple typed values and to pass resolver for QName values | ||
| public override void WriteValue(object value) |
There was a problem hiding this comment.
Whole logic for writing with buffer is duplicated in XmlTextWriter and XmlRawWriter.
One suggestion is to create "public" class( this is needed as XmlTextWriter is public) and encapsulate logic in it.
The other is to have some static helper class which works by ref with char[].
Which one do you prefer ?
There was a problem hiding this comment.
Duplication logic reduced, but the question still stays open.
|
@krwq, @eiriktsarpalis Is it possible to receive some initial feedback on these changes and to the following ones(changes in serializators to use typed methods instead of converting to string first) ? |
| ArgumentNullException.ThrowIfNull(value); | ||
|
|
||
| WriteString(XmlUntypedConverter.Untyped.ToString(value, _resolver)); | ||
| Type sourceType = value.GetType(); |
There was a problem hiding this comment.
This seems to be adding a lot of branching where it previously didn't exist. Could this be done differently? I'm not super familiar with this code, but one thought might be to expose a TryFormat method to XmlUntypedConverter.Untyped.
There was a problem hiding this comment.
The code in XmlUntypedConverter.ToString(object) is already similar:
```cs
public override string ToString(object value, IXmlNamespaceResolver? nsResolver)
{
ArgumentNullException.ThrowIfNull(value);
Type sourceType = value.GetType();
if (sourceType == BooleanType) return XmlConvert.ToString((bool)value);
if (sourceType == ByteType) return XmlConvert.ToString((byte)value);
if (sourceType == ByteArrayType) return Base64BinaryToString((byte[])value);
if (sourceType == DateTimeType) return DateTimeToString((DateTime)value);
if (sourceType == DateTimeOffsetType) return DateTimeOffsetToString((DateTimeOffset)value);
if (sourceType == DecimalType) return XmlConvert.ToString((decimal)value);
if (sourceType == DoubleType) return XmlConvert.ToString((double)value);
if (sourceType == Int16Type) return XmlConvert.ToString((short)value);
if (sourceType == Int32Type) return XmlConvert.ToString((int)value);
if (sourceType == Int64Type) return XmlConvert.ToString((long)value);
if (sourceType == SByteType) return XmlConvert.ToString((sbyte)value);
if (sourceType == SingleType) return XmlConvert.ToString((float)value);
if (sourceType == StringType) return ((string)value);
if (sourceType == TimeSpanType) return DurationToString((TimeSpan)value);
if (sourceType == UInt16Type) return XmlConvert.ToString((ushort)value);
if (sourceType == UInt32Type) return XmlConvert.ToString((uint)value);
if (sourceType == UInt64Type) return XmlConvert.ToString((ulong)value);
if (IsDerivedFrom(sourceType, UriType)) return AnyUriToString((Uri)value);
if (sourceType == XmlAtomicValueType) return ((string)((XmlAtomicValue)value).ValueAs(StringType, nsResolver));
if (IsDerivedFrom(sourceType, XmlQualifiedNameType)) return QNameToString((XmlQualifiedName)value, nsResolver);
return (string)ChangeTypeWildcardDestination(value, StringType, nsResolver);
}
src/libraries/System.Private.Xml/src/System/Xml/Core/XmlTextWriter.cs
Outdated
Show resolved
Hide resolved
|
Closing this in favor of 76436 |
This PR suggest using small char[] buffer in XmlRawWriter and use it for formatting primitive types in it. Then using WriteRaw(char[]) method instead of creating temporary string and WriteRaw(string) method.
Also exposing XmlConverter.TryFormat methods for already exposed ToString(xxx) primitive types, allowing writes to Span destination.
Last change is to expose TryFormat in XsdDateTime and XsdDuration.
This PR will open possibilities in XmlSerializers to use typed methods of XmlWriter instead of always fallback to WriteString one.
Sample optimization