Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add char[] buffer to XmlRawWriter. #75411

152 changes: 137 additions & 15 deletions src/libraries/System.Private.Xml/src/System/Xml/Core/XmlRawWriter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Xml.XPath;
using System.Xml.Schema;
using System.Collections;
using System.Xml.Serialization;

namespace System.Xml
{
Expand Down Expand Up @@ -47,6 +48,9 @@ internal abstract partial class XmlRawWriter : XmlWriter
// namespace resolver
protected IXmlNamespaceResolver? _resolver;

private char[] _primitivesBuffer = new char[36];
TrayanZapryanov marked this conversation as resolved.
Show resolved Hide resolved
private static readonly TypeScope s_typeScope = new TypeScope();

//
// XmlWriter implementation
//
Expand Down Expand Up @@ -143,17 +147,19 @@ public override void WriteCData(string? text)
WriteString(text);
}

// Forward call to WriteString(string).
// Forward call to WriteChars.
public override void WriteCharEntity(char ch)
{
WriteString(char.ToString(ch));
_primitivesBuffer[0] = ch;
TrayanZapryanov marked this conversation as resolved.
Show resolved Hide resolved
WriteChars(_primitivesBuffer, 0, 1);
}

// Forward call to WriteString(string).
// Forward call to WriteChars.
public override void WriteSurrogateCharEntity(char lowChar, char highChar)
{
ReadOnlySpan<char> entity = stackalloc char[] { lowChar, highChar };
WriteString(new string(entity));
_primitivesBuffer[0] = lowChar;
_primitivesBuffer[1] = highChar;
WriteChars(_primitivesBuffer, 0, 2);
}

// Forward call to WriteString(string).
Expand All @@ -162,16 +168,10 @@ public override void WriteWhitespace(string? ws)
WriteString(ws);
}

// Forward call to WriteString(string).
// Forward call to WriteRaw(char[]).
public override void WriteChars(char[] buffer, int index, int count)
{
WriteString(new string(buffer, index, count));
}

// Forward call to WriteString(string).
public override void WriteRaw(char[] buffer, int index, int count)
{
WriteString(new string(buffer, index, count));
WriteRaw(buffer, index, count);
}

// Forward call to WriteString(string).
Expand All @@ -185,7 +185,74 @@ public override void WriteValue(object value)
{
ArgumentNullException.ThrowIfNull(value);

WriteString(XmlUntypedConverter.Untyped.ToString(value, _resolver));
Type sourceType = value.GetType();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);
}

if (!TypeScope.IsKnownType(sourceType))
{
WriteString(XmlUntypedConverter.Untyped.ToString(value, _resolver));
return;
}

#pragma warning disable IL2026
TrayanZapryanov marked this conversation as resolved.
Show resolved Hide resolved
TypeDesc typeDesc = s_typeScope.GetTypeDesc(sourceType);
#pragma warning restore IL2026
switch (typeDesc.FormatterName)
{
case "Boolean":
WriteWithBuffer((bool)value, XmlConvert.TryFormat);
break;
case "Int32":
WriteWithBuffer((int)value, XmlConvert.TryFormat);
break;
case "Int16":
WriteWithBuffer((short)value, XmlConvert.TryFormat);
break;
case "Int64":
WriteWithBuffer((long)value, XmlConvert.TryFormat);
break;
case "Single":
WriteWithBuffer((float)value, XmlConvert.TryFormat);
break;
case "Double":
WriteWithBuffer((double)value, XmlConvert.TryFormat);
break;
case "Decimal":
WriteWithBuffer((decimal)value, XmlConvert.TryFormat);
break;
case "Byte":
WriteWithBuffer((byte)value, XmlConvert.TryFormat);
break;
case "SByte":
WriteWithBuffer((sbyte)value, XmlConvert.TryFormat);
break;
case "UInt16":
WriteWithBuffer((ushort)value, XmlConvert.TryFormat);
break;
case "UInt32":
WriteWithBuffer((uint)value, XmlConvert.TryFormat);
break;
case "UInt64":
WriteWithBuffer((ulong)value, XmlConvert.TryFormat);
break;
//Guid is not supported in XmlUntypedConverter.Untyped and throws exception
TrayanZapryanov marked this conversation as resolved.
Show resolved Hide resolved
//case "Guid":
// WriteWithBuffer((Guid)value, XmlConvert.TryFormat);
// break;
case "Char":
WriteWithBuffer((char)value, XmlConvert.TryFormat);
break;
case "TimeSpan":
WriteWithBuffer((TimeSpan)value, XmlConvert.TryFormat);
break;
case "DateTime":
WriteWithBuffer((DateTime)value, XmlConvert.TryFormat);
break;
case "DateTimeOffset":
WriteWithBuffer((DateTimeOffset)value, XmlConvert.TryFormat);
break;
default:
WriteString(XmlUntypedConverter.Untyped.ToString(value, _resolver));
break;
}
}

// Override in order to handle Xml simple typed values and to pass resolver for QName values
Expand All @@ -198,7 +265,7 @@ public override void WriteValue(DateTimeOffset value)
{
// 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));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment seems not relevant as even now it is using

public static string ToString(DateTimeOffset value)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it is connected with base code ?

public virtual void WriteValue(DateTimeOffset value)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@krwq Do you know if somebody is changing this Mode ?

internal static class XmlCustomFormatter
{
private static DateTimeSerializationSection.DateTimeSerializationMode s_mode;
private static DateTimeSerializationSection.DateTimeSerializationMode Mode
{
get
{
if (s_mode == DateTimeSerializationSection.DateTimeSerializationMode.Default)
{
s_mode = DateTimeSerializationSection.DateTimeSerializationMode.Roundtrip;
}
return s_mode;
}
}

I saw similar code in XmlSerializers and the only way to change is with Reflection and directly manipulating fiedl.

WriteWithBuffer(value, XmlConvert.TryFormat);
}

// Copying to XmlRawWriter is not currently supported.
Expand All @@ -217,6 +284,41 @@ public override void WriteNode(System.Xml.XPath.XPathNavigator navigator, bool d
throw new InvalidOperationException(SR.Xml_InvalidOperation);
}

public override void WriteValue(bool value)
{
WriteWithBuffer(value, XmlConvert.TryFormat);
}

public override void WriteValue(int value)
{
WriteWithBuffer(value, XmlConvert.TryFormat);
}

public override void WriteValue(long value)
{
WriteWithBuffer(value, XmlConvert.TryFormat);
}

public override void WriteValue(double value)
{
WriteWithBuffer(value, XmlConvert.TryFormat);
}

public override void WriteValue(float value)
{
WriteWithBuffer(value, XmlConvert.TryFormat);
}

public override void WriteValue(decimal value)
{
WriteWithBuffer(value, XmlConvert.TryFormat);
}

public override void WriteValue(DateTime value)
{
WriteWithBuffer(value, XmlConvert.TryFormat);
}

//
// XmlRawWriter methods and properties
//
Expand Down Expand Up @@ -315,5 +417,25 @@ internal virtual void Close(WriteState currentState)
{
Close();
}

private delegate bool GrowPrimitiveBufferDelegate<in T>(T value, Span<char> destination, out int charsWritten);
private void WriteWithBuffer<T>(T value, GrowPrimitiveBufferDelegate<T> checkFunc)
{
ArgumentNullException.ThrowIfNull(value);

//fast path without loop
if (checkFunc(value, _primitivesBuffer, out int charsWritten))
{
WriteChars(_primitivesBuffer, 0, charsWritten);
return;
}

while (!checkFunc(value, _primitivesBuffer, out charsWritten))
{
_primitivesBuffer = new char[_primitivesBuffer.Length * 2];
}

WriteChars(_primitivesBuffer, 0, charsWritten);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw that there is difference between

public override unsafe void WriteChars(char[] buffer, int index, int count)

and

public override unsafe void WriteRaw(char[] buffer, int index, int count)

Which one should be used ?

}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,8 @@ private enum XsdDateTimeKind
private static readonly int[] DaysToMonth366 = {
0, 31, 60, 91, 121, 152, 182, 213, 244, 274, 305, 335, 366};

private const int CharStackBufferSize = 64;

/// <summary>
/// Constructs an XsdDateTime from a string using specific format.
/// </summary>
Expand Down Expand Up @@ -494,7 +496,22 @@ public static implicit operator DateTimeOffset(XsdDateTime xdt)
/// </summary>
public override string ToString()
{
var vsb = new ValueStringBuilder(stackalloc char[64]);
var vsb = new ValueStringBuilder(stackalloc char[CharStackBufferSize]);
Format(ref vsb);

return vsb.ToString();
}

public bool TryFormat(Span<char> destination, out int charsWritten)
{
var sb = new ValueStringBuilder(stackalloc char[CharStackBufferSize]);
Format(ref sb);

return sb.TryCopyTo(destination, out charsWritten);
}

private void Format(ref ValueStringBuilder vsb)
{
switch (InternalTypeCode)
{
case DateTimeTypeCode.DateTime:
Expand Down Expand Up @@ -533,7 +550,6 @@ public override string ToString()
break;
}
PrintZone(ref vsb);
return vsb.ToString();
}

// Serialize year, month and day
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ internal struct XsdDuration
private uint _nanoseconds; // High bit is used to indicate whether duration is negative

private const uint NegativeBit = 0x80000000;
private const int CharStackBufferSize = 20;

private enum Parts
{
Expand Down Expand Up @@ -334,13 +335,28 @@ public override string ToString()
return ToString(DurationType.Duration);
}

public bool TryFormat(Span<char> destination, out int charsWritten, DurationType durationType = DurationType.Duration)
{
var sb = new ValueStringBuilder(stackalloc char[CharStackBufferSize]);
Format(ref sb, durationType);

return sb.TryCopyTo(destination, out charsWritten);
}

/// <summary>
/// Return the string representation according to xsd:duration rules, xdt:dayTimeDuration rules, or
/// xdt:yearMonthDuration rules.
/// </summary>
internal string ToString(DurationType durationType)
{
var vsb = new ValueStringBuilder(stackalloc char[20]);
var vsb = new ValueStringBuilder(stackalloc char[CharStackBufferSize]);
Format(ref vsb, durationType);

return vsb.ToString();
}

private void Format(ref ValueStringBuilder vsb, DurationType durationType)
{
int nanoseconds, digit, zeroIdx, len;

if (IsNegative)
Expand Down Expand Up @@ -410,7 +426,8 @@ internal string ToString(DurationType durationType)
}

vsb.EnsureCapacity(zeroIdx + 1);
vsb.Append(tmpSpan.Slice(0, zeroIdx - len + 1));
var nanoSpanLength = zeroIdx - len + 1;
tmpSpan[..nanoSpanLength].TryCopyTo(vsb.AppendSpan(nanoSpanLength));
}
vsb.Append('S');
}
Expand All @@ -426,8 +443,6 @@ internal string ToString(DurationType durationType)
if (vsb[vsb.Length - 1] == 'P')
vsb.Append("0M");
}

return vsb.ToString();
}

internal static Exception? TryParse(string s, out XsdDuration result)
Expand Down
Loading