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 <[email protected]>

Author: MichalPetryka

src/libraries/System.Data.Common/ref/System.Data.Common.cs

@@ -3224,9 +3224,8 @@ void System.Xml.Serialization.IXmlSerializable.WriteXml(System.Xml.XmlWriter wri
         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<System.Data.SqlTypes.SqlGuid>
+    public partial struct SqlGuid : System.Data.SqlTypes.INullable, System.IComparable, System.Runtime.Serialization.ISerializable, System.Xml.Serialization.IXmlSerializable, System.IEquatable<System.Data.SqlTypes.SqlGuid>
     {
-        private object _dummy;
         private int _dummyPrimitive;
         public static readonly System.Data.SqlTypes.SqlGuid Null;
         public SqlGuid(byte[] value) { throw null; }
@@ -3261,6 +3260,7 @@ public partial struct SqlGuid : System.Data.SqlTypes.INullable, System.IComparab
         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; }

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,71 +20,60 @@ 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<SqlGuid>
+    public struct SqlGuid : INullable, IComparable, ISerializable, IXmlSerializable, IEquatable<SqlGuid>
     {
-        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)
             : this(new Guid(a, b, c, d, e, f, g, h, i, j, k))
         {
         }
 
-
-        // 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 @@ public static explicit operator Guid(SqlGuid x)
 
         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 @@ public static SqlGuid Parse(string s)
                 return new SqlGuid(s);
         }
 
-
         // Comparison operators
         private static EComparison Compare(SqlGuid x, SqlGuid y)
         {
             // Comparison orders.
-            ReadOnlySpan<byte> rgiGuidOrder = new byte[16] { 10, 11, 12, 13, 14, 15, 8, 9, 6, 7, 4, 5, 0, 1, 2, 3 };
+            ReadOnlySpan<byte> 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<byte> xBytes = x.m_value;
-            ReadOnlySpan<byte> yBytes = y.m_value;
+            Span<byte> xBytes = stackalloc byte[SizeOfGuid];
+            bool xWrote = x._value.GetValueOrDefault().TryWriteBytes(xBytes);
+            Debug.Assert(xWrote);
+
+            Span<byte> 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 @@ private static EComparison Compare(SqlGuid x, SqlGuid y)
             return EComparison.EQ;
         }
 
-
-
         // Implicit conversions
 
         // Explicit conversions
@@ -161,7 +159,8 @@ public static explicit operator SqlGuid(SqlBinary x)
         // 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 @@ public SqlBinary ToSqlBinary()
             return (SqlBinary)this;
         }
 
-
         // IComparable
         // Compares this object to another object, returning an integer that
         // indicates the relationship.
@@ -287,12 +285,10 @@ public override bool Equals([NotNullWhen(true)] object? value) =>
         /// <summary>Indicates whether the current instance is equal to another instance of the same type.</summary>
         /// <param name="other">An instance to compare with this instance.</param>
         /// <returns>true if the current instance is equal to the other instance; otherwise, false.</returns>
-        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 @@ void IXmlSerializable.ReadXml(XmlReader reader)
             {
                 // 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 @@ public static XmlQualifiedName GetXsdType(XmlSchemaSet schemaSet)
             return new XmlQualifiedName("string", XmlSchema.Namespace);
         }
 
-        public static readonly SqlGuid Null = new SqlGuid(true);
+        public static readonly SqlGuid Null;
     } // SqlGuid
 } // namespace System.Data.SqlTypes