From bf5afd4480615ee5c7e114669c7a14eea1436348 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Micha=C5=82=20Petryka?= <35800402+MichalPetryka@users.noreply.github.com> Date: Mon, 30 Jan 2023 18:35:49 +0100 Subject: [PATCH] Optimize the SqlGuid struct (#72549) * Optimize the SqlGuid struct Optimizes the SqlGuid struct according to the idea from #51836. * Update SQLGuid.cs * Apply suggestions * Update SQLGuid.cs * Update SQLGuid.cs * Update ref assembly * Update System.Data.Common.cs * Apply suggestions * Update SQLGuid.cs * Update SQLGuid.cs * Update SQLGuid.cs * Add a comment about binary serialization * Expand on the comment for backwards-compatible binary serialization --------- Co-authored-by: Jeff Handley --- .../System.Data.Common/ref/System.Data.Common.cs | 4 +- .../src/System/Data/SQLTypes/SQLGuid.cs | 110 ++++++++++----------- 2 files changed, 55 insertions(+), 59 deletions(-) diff --git a/src/libraries/System.Data.Common/ref/System.Data.Common.cs b/src/libraries/System.Data.Common/ref/System.Data.Common.cs index 0ea1f0a..c8565ae 100644 --- a/src/libraries/System.Data.Common/ref/System.Data.Common.cs +++ b/src/libraries/System.Data.Common/ref/System.Data.Common.cs @@ -3224,9 +3224,8 @@ namespace System.Data.SqlTypes public override string ToString() { throw null; } } [System.Xml.Serialization.XmlSchemaProviderAttribute("GetXsdType")] - public partial struct SqlGuid : System.Data.SqlTypes.INullable, System.IComparable, System.Xml.Serialization.IXmlSerializable, System.IEquatable + public partial struct SqlGuid : System.Data.SqlTypes.INullable, System.IComparable, System.Runtime.Serialization.ISerializable, System.Xml.Serialization.IXmlSerializable, System.IEquatable { - private object _dummy; private int _dummyPrimitive; public static readonly System.Data.SqlTypes.SqlGuid Null; public SqlGuid(byte[] value) { throw null; } @@ -3261,6 +3260,7 @@ namespace System.Data.SqlTypes System.Xml.Schema.XmlSchema System.Xml.Serialization.IXmlSerializable.GetSchema() { throw null; } void System.Xml.Serialization.IXmlSerializable.ReadXml(System.Xml.XmlReader reader) { } void System.Xml.Serialization.IXmlSerializable.WriteXml(System.Xml.XmlWriter writer) { } + void System.Runtime.Serialization.ISerializable.GetObjectData(System.Runtime.Serialization.SerializationInfo info, System.Runtime.Serialization.StreamingContext context) { } public byte[]? ToByteArray() { throw null; } public System.Data.SqlTypes.SqlBinary ToSqlBinary() { throw null; } public System.Data.SqlTypes.SqlString ToSqlString() { throw null; } diff --git a/src/libraries/System.Data.Common/src/System/Data/SQLTypes/SQLGuid.cs b/src/libraries/System.Data.Common/src/System/Data/SQLTypes/SQLGuid.cs index fab4a39..291a169 100644 --- a/src/libraries/System.Data.Common/src/System/Data/SQLTypes/SQLGuid.cs +++ b/src/libraries/System.Data.Common/src/System/Data/SQLTypes/SQLGuid.cs @@ -2,6 +2,10 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Data.Common; +using System.Diagnostics; +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; +using System.Runtime.Serialization; using System.Xml; using System.Xml.Schema; using System.Xml.Serialization; @@ -16,45 +20,30 @@ namespace System.Data.SqlTypes [Serializable] [XmlSchemaProvider("GetXsdType")] [System.Runtime.CompilerServices.TypeForwardedFrom("System.Data, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089")] - public struct SqlGuid : INullable, IComparable, IXmlSerializable, IEquatable + public struct SqlGuid : INullable, IComparable, ISerializable, IXmlSerializable, IEquatable { - private const int SizeOfGuid = 16; + private const int SizeOfGuid = 16; // sizeof(Guid) // NOTE: If any instance fields change, update SqlTypeWorkarounds type in System.Data.SqlClient. - private byte[]? m_value; // the SqlGuid is null if m_value is null + private Guid? _value; // the SqlGuid is null if _value is null // constructor - // construct a SqlGuid.Null - private SqlGuid(bool _) - { - m_value = null; - } - public SqlGuid(byte[] value) { if (value == null || value.Length != SizeOfGuid) throw new ArgumentException(SQLResource.InvalidArraySizeMessage); - m_value = new byte[SizeOfGuid]; - value.CopyTo(m_value, 0); - } - - internal SqlGuid(byte[] value, bool _) - { - if (value == null || value.Length != SizeOfGuid) - throw new ArgumentException(SQLResource.InvalidArraySizeMessage); - - m_value = value; + _value = new Guid(value); } public SqlGuid(string s) { - m_value = (new Guid(s)).ToByteArray(); + _value = new Guid(s); } public SqlGuid(Guid g) { - m_value = g.ToByteArray(); + _value = g; } public SqlGuid(int a, short b, short c, byte d, byte e, byte f, byte g, byte h, byte i, byte j, byte k) @@ -62,25 +51,29 @@ namespace System.Data.SqlTypes { } - - // INullable - public bool IsNull + // Maintains backwards-compatible binary serialization for the `private byte[] m_value` field + // see src/libraries/System.Runtime.Serialization.Formatters/tests/BinaryFormatterTestData.cs + // for test data + private SqlGuid(SerializationInfo info, StreamingContext context) { - get { return (m_value is null); } + byte[]? value = (byte[]?)info.GetValue("m_value", typeof(byte[])); + if (value is null) + _value = null; + else + _value = new Guid(value); } - // property: Value - public Guid Value + void ISerializable.GetObjectData(SerializationInfo info, StreamingContext context) { - get - { - if (m_value is null) - throw new SqlNullValueException(); - else - return new Guid(m_value); - } + info.AddValue("m_value", ToByteArray(), typeof(byte[])); } + // INullable + public bool IsNull => _value is null; + + // property: Value + public Guid Value => _value ?? throw new SqlNullValueException(); + // Implicit conversion from Guid to SqlGuid public static implicit operator SqlGuid(Guid x) { @@ -95,18 +88,17 @@ namespace System.Data.SqlTypes public byte[]? ToByteArray() { - byte[] ret = new byte[SizeOfGuid]; - m_value!.CopyTo(ret, 0); // TODO: NRE - return ret; + if (_value is null) + return null; + return _value.GetValueOrDefault().ToByteArray(); } public override string ToString() { - if (m_value is null) + if (_value is null) return SQLResource.NullString; - Guid g = new Guid(m_value); - return g.ToString(); + return _value.GetValueOrDefault().ToString(); } public static SqlGuid Parse(string s) @@ -117,16 +109,24 @@ namespace System.Data.SqlTypes return new SqlGuid(s); } - // Comparison operators private static EComparison Compare(SqlGuid x, SqlGuid y) { // Comparison orders. - ReadOnlySpan rgiGuidOrder = new byte[16] { 10, 11, 12, 13, 14, 15, 8, 9, 6, 7, 4, 5, 0, 1, 2, 3 }; + ReadOnlySpan rgiGuidOrder = new byte[SizeOfGuid] { 10, 11, 12, 13, 14, 15, 8, 9, 6, 7, 4, 5, 0, 1, 2, 3 }; + + Debug.Assert(!x.IsNull); + Debug.Assert(!y.IsNull); // Swap to the correct order to be compared - ReadOnlySpan xBytes = x.m_value; - ReadOnlySpan yBytes = y.m_value; + Span xBytes = stackalloc byte[SizeOfGuid]; + bool xWrote = x._value.GetValueOrDefault().TryWriteBytes(xBytes); + Debug.Assert(xWrote); + + Span yBytes = stackalloc byte[SizeOfGuid]; + bool yWrote = y._value.GetValueOrDefault().TryWriteBytes(yBytes); + Debug.Assert(yWrote); + for (int i = 0; i < SizeOfGuid; i++) { byte b1 = xBytes[rgiGuidOrder[i]]; @@ -140,8 +140,6 @@ namespace System.Data.SqlTypes return EComparison.EQ; } - - // Implicit conversions // Explicit conversions @@ -161,7 +159,8 @@ namespace System.Data.SqlTypes // Overloading comparison operators public static SqlBoolean operator ==(SqlGuid x, SqlGuid y) { - return (x.IsNull || y.IsNull) ? SqlBoolean.Null : new SqlBoolean(Compare(x, y) == EComparison.EQ); + return (x.IsNull || y.IsNull) ? SqlBoolean.Null : + new SqlBoolean(x._value.GetValueOrDefault() == y._value.GetValueOrDefault()); } public static SqlBoolean operator !=(SqlGuid x, SqlGuid y) @@ -249,7 +248,6 @@ namespace System.Data.SqlTypes return (SqlBinary)this; } - // IComparable // Compares this object to another object, returning an integer that // indicates the relationship. @@ -287,12 +285,10 @@ namespace System.Data.SqlTypes /// Indicates whether the current instance is equal to another instance of the same type. /// An instance to compare with this instance. /// true if the current instance is equal to the other instance; otherwise, false. - public bool Equals(SqlGuid other) => - other.IsNull || IsNull ? other.IsNull && IsNull : - (this == other).Value; + public bool Equals(SqlGuid other) => _value == other._value; // For hashing purpose - public override int GetHashCode() => IsNull ? 0 : Value.GetHashCode(); + public override int GetHashCode() => _value.GetHashCode(); XmlSchema? IXmlSerializable.GetSchema() { return null; } @@ -303,23 +299,23 @@ namespace System.Data.SqlTypes { // Read the next value. reader.ReadElementString(); - m_value = null; + _value = null; } else { - m_value = new Guid(reader.ReadElementString()).ToByteArray(); + _value = new Guid(reader.ReadElementString()); } } void IXmlSerializable.WriteXml(XmlWriter writer) { - if (m_value is null) + if (_value is null) { writer.WriteAttributeString("xsi", "nil", XmlSchema.InstanceNamespace, "true"); } else { - writer.WriteString(XmlConvert.ToString(new Guid(m_value))); + writer.WriteString(XmlConvert.ToString(_value.GetValueOrDefault())); } } @@ -328,6 +324,6 @@ namespace System.Data.SqlTypes return new XmlQualifiedName("string", XmlSchema.Namespace); } - public static readonly SqlGuid Null = new SqlGuid(true); + public static readonly SqlGuid Null; } // SqlGuid } // namespace System.Data.SqlTypes -- 2.7.4