Adding 'LanguagePrimitives.TryCompare' to provide faster comparisons (P…

…owerShell#7438)

src/Microsoft.PowerShell.Commands.Utility/commands/utility/ObjectCommandComparer.cs

@@ -79,31 +79,30 @@ public override bool Equals(Object inputObject)
         {
             ObjectCommandPropertyValue objectCommandPropertyValueObject = inputObject as ObjectCommandPropertyValue;
             if (objectCommandPropertyValueObject == null)
+            {
                 return false;
+            }
 
             object baseObject = PSObject.Base(PropertyValue);
             object inComingbaseObjectPropertyValue = PSObject.Base(objectCommandPropertyValueObject.PropertyValue);
 
-            IComparable baseObjectComparable = baseObject as IComparable;
+            if (baseObject is IComparable)
+            {
+                var success = LanguagePrimitives.TryCompare(baseObject, inComingbaseObjectPropertyValue, CaseSensitive, Culture, out int result);
+                return success && result == 0;
+            }
 
-            if (baseObjectComparable != null)
+            if (baseObject == null && inComingbaseObjectPropertyValue == null)
             {
-                return (LanguagePrimitives.Compare(baseObject, inComingbaseObjectPropertyValue, CaseSensitive, Culture) == 0);
+                return true;
             }
-            else
+            if (baseObject != null && inComingbaseObjectPropertyValue != null)
             {
-                if (baseObject == null && inComingbaseObjectPropertyValue == null)
-                {
-                    return true;
-                }
-                if (baseObject != null && inComingbaseObjectPropertyValue != null)
-                {
-                    return baseObject.ToString().Equals(inComingbaseObjectPropertyValue.ToString(), StringComparison.OrdinalIgnoreCase);
-                }
-
-                // One of the property values being compared is null.
-                return false;
+                return baseObject.ToString().Equals(inComingbaseObjectPropertyValue.ToString(), StringComparison.OrdinalIgnoreCase);
             }
+
+            // One of the property values being compared is null.
+            return false;
         }
 
         /// <summary>
@@ -190,35 +189,30 @@ public int Compare(object first, object second)
         {
             // This method will never throw exceptions, two null
             // objects are considered the same
-            if (IsValueNull(first) && IsValueNull(second)) return 0;
+            if (IsValueNull(first) && IsValueNull(second))
+            {
+                return 0;
+            }
 
-            PSObject firstMsh = first as PSObject;
-            if (firstMsh != null)
+            if (first is PSObject firstMsh)
             {
                 first = firstMsh.BaseObject;
             }
 
-            PSObject secondMsh = second as PSObject;
-            if (secondMsh != null)
+            if (second is PSObject secondMsh)
             {
                 second = secondMsh.BaseObject;
             }
 
-            try
-            {
-                return LanguagePrimitives.Compare(first, second, !_caseSensitive, _cultureInfo) * (_ascendingOrder ? 1 : -1);
-            }
-            catch (InvalidCastException)
-            {
-            }
-            catch (ArgumentException)
+            if (LanguagePrimitives.TryCompare(first, second, !_caseSensitive, _cultureInfo, out int result))
             {
-                // Note that this will occur if the objects do not support
-                // IComparable.  We fall back to comparing as strings.
+                return result * (_ascendingOrder ? 1 : -1);
             }
 
+            // Note that this will occur if the objects do not support
+            // IComparable.  We fall back to comparing as strings.
+
             // being here means the first object doesn't support ICompare
-            // or an Exception was raised win Compare
             string firstString = PSObject.AsPSObject(first).ToString();
             string secondString = PSObject.AsPSObject(second).ToString();
 

src/System.Management.Automation/FormatAndOutput/common/FormatGroupManager.cs

@@ -83,19 +83,14 @@ internal bool UpdateGroupingKeyValue(PSObject so)
 
         private static bool IsEqual(object first, object second)
         {
-            try
+            if (LanguagePrimitives.TryCompare(first, second, true, CultureInfo.CurrentCulture, out int result))
             {
-                return LanguagePrimitives.Compare(first, second, true, CultureInfo.CurrentCulture) == 0;
-            }
-            catch (InvalidCastException)
-            {
-            }
-            catch (ArgumentException)
-            {
-                // Note that this will occur if the objects do not support
-                // IComparable.  We fall back to comparing as strings.
+                return result == 0;
             }
 
+            // Note that this will occur if the objects do not support
+            // IComparable.  We fall back to comparing as strings.
+
             // being here means the first object doesn't support ICompare
             // or an Exception was raised win Compare
             string firstString = PSObject.AsPSObject(first).ToString();

src/System.Management.Automation/engine/LanguagePrimitives.cs

@@ -695,6 +695,30 @@ public static bool Equals(object first, object second, bool ignoreCase, IFormatP
             return false;
         }
 
+        /// <summary>
+        /// Helper method for [Try]Compare to determine object ordering with null.
+        /// </summary>
+        /// <param name="value">the numeric value to compare to null</param>
+        /// <param name="numberIsRightHandSide">True if the number to compare is on the right hand side if the comparison.</param>
+        private static int CompareObjectToNull(object value, bool numberIsRightHandSide)
+        {
+            var i = numberIsRightHandSide ? -1 : 1;
+
+            // If it's a positive number, including 0, it's greater than null
+            // for everything else it's less than zero...
+            switch (value)
+            {
+                case Int16 i16: return Math.Sign(i16) < 0 ? -i : i;
+                case Int32 i32: return Math.Sign(i32) < 0 ? -i : i;
+                case Int64 i64: return Math.Sign(i64) < 0 ? -i : i;
+                case sbyte sby: return Math.Sign(sby) < 0 ? -i : i;
+                case float f: return Math.Sign(f) < 0 ? -i : i;
+                case double d: return Math.Sign(d) < 0 ? -i : i;
+                case decimal de: return Math.Sign(de) < 0 ? -i : i;
+                default: return i;
+            }
+        }
+
         /// <summary>
         /// Compare first and second, converting second to the
         /// type of the first, if necessary.
@@ -762,43 +786,12 @@ public static int Compare(object first, object second, bool ignoreCase, IFormatP
 
             if (first == null)
             {
-                if (second == null)
-                {
-                    return 0;
-                }
-                else
-                {
-                    // If it's a positive number, including 0, it's greater than null
-                    // for everything else it's less than zero...
-                    switch (LanguagePrimitives.GetTypeCode(second.GetType()))
-                    {
-                        case TypeCode.Int16: return System.Math.Sign((Int16)second) < 0 ? 1 : -1;
-                        case TypeCode.Int32: return System.Math.Sign((Int32)second) < 0 ? 1 : -1;
-                        case TypeCode.Int64: return System.Math.Sign((Int64)second) < 0 ? 1 : -1;
-                        case TypeCode.SByte: return System.Math.Sign((sbyte)second) < 0 ? 1 : -1;
-                        case TypeCode.Single: return System.Math.Sign((System.Single)second) < 0 ? 1 : -1;
-                        case TypeCode.Double: return System.Math.Sign((System.Double)second) < 0 ? 1 : -1;
-                        case TypeCode.Decimal: return System.Math.Sign((System.Decimal)second) < 0 ? 1 : -1;
-                        default: return -1;
-                    }
-                }
+                return second == null ? 0 : CompareObjectToNull(second, true);
             }
 
             if (second == null)
             {
-                // If it's a positive number, including 0, it's greater than null
-                // for everything else it's less than zero...
-                switch (LanguagePrimitives.GetTypeCode(first.GetType()))
-                {
-                    case TypeCode.Int16: return System.Math.Sign((Int16)first) < 0 ? -1 : 1;
-                    case TypeCode.Int32: return System.Math.Sign((Int32)first) < 0 ? -1 : 1;
-                    case TypeCode.Int64: return System.Math.Sign((Int64)first) < 0 ? -1 : 1;
-                    case TypeCode.SByte: return System.Math.Sign((sbyte)first) < 0 ? -1 : 1;
-                    case TypeCode.Single: return System.Math.Sign((System.Single)first) < 0 ? -1 : 1;
-                    case TypeCode.Double: return System.Math.Sign((System.Double)first) < 0 ? -1 : 1;
-                    case TypeCode.Decimal: return System.Math.Sign((System.Decimal)first) < 0 ? -1 : 1;
-                    default: return 1;
-                }
+                return CompareObjectToNull(first, false);
             }
 
             if (first is string firstString)
@@ -854,6 +847,127 @@ public static int Compare(object first, object second, bool ignoreCase, IFormatP
             throw PSTraceSource.NewArgumentException("first", ExtendedTypeSystem.NotIcomparable, first.ToString());
         }
 
+        /// <summary>
+        /// Tries to compare first and second, converting second to the type of the first, if necessary.
+        /// If a conversion is needed but fails, false is return.
+        /// </summary>
+        /// <param name="first">First comparison value.</param>
+        /// <param name="second">Second comparison value.</param>
+        /// <param name="result">Less than zero if first is smaller than second, more than
+        /// zero if it is greater or zero if they are the same.</param>
+        /// <returns>True if the comparison was successful, false otherwise.</returns>
+        public static bool TryCompare(object first, object second, out int result)
+        {
+            return TryCompare(first, second, ignoreCase: false, CultureInfo.InvariantCulture, out result);
+        }
+
+        /// <summary>
+        /// Tries to compare first and second, converting second to the type of the first, if necessary.
+        /// If a conversion is needed but fails, false is return.
+        /// </summary>
+        /// <param name="first">First comparison value.</param>
+        /// <param name="second">Second comparison value.</param>
+        /// <param name="ignoreCase">Used if both values are strings.</param>
+        /// <param name="result">Less than zero if first is smaller than second, more than zero if it is greater or zero if they are the same.</param>
+        /// <returns>True if the comparison was successful, false otherwise.</returns>
+        public static bool TryCompare(object first, object second, bool ignoreCase, out int result)
+        {
+            return TryCompare(first, second, ignoreCase, CultureInfo.InvariantCulture, out result);
+        }
+
+        /// <summary>
+        /// Tries to compare first and second, converting second to the type of the first, if necessary.
+        /// If a conversion is needed but fails, false is return.
+        /// </summary>
+        /// <param name="first">First comparison value.</param>
+        /// <param name="second">Second comparison value.</param>
+        /// <param name="ignoreCase">Used if both values are strings.</param>
+        /// <param name="formatProvider">Used in type conversions and if both values are strings.</param>
+        /// <param name="result">Less than zero if first is smaller than second, more than  zero if it is greater or zero if they are the same.</param>
+        /// <returns>True if the comparison was successful, false otherwise.</returns>
+        /// <exception cref="ArgumentException">The parameter <paramref name="formatProvider"/> is not a <see cref="CultureInfo"/>.</exception>
+        public static bool TryCompare(object first, object second, bool ignoreCase, IFormatProvider formatProvider, out int result)
+        {
+            result = 0;
+            if (formatProvider == null)
+            {
+                formatProvider = CultureInfo.InvariantCulture;
+            }
+
+            if (!(formatProvider is CultureInfo culture))
+            {
+                throw PSTraceSource.NewArgumentException("formatProvider");
+            }
+
+            first = PSObject.Base(first);
+            second = PSObject.Base(second);
+
+            if (first == null && second == null)
+            {
+                result = 0;
+                return true;
+            }
+
+            if (first == null)
+            {
+                result = CompareObjectToNull(second, true);
+                return true;
+            }
+
+            if (second == null)
+            {
+                // If it's a positive number, including 0, it's greater than null
+                // for everything else it's less than zero...
+                result = CompareObjectToNull(first, false);
+                return true;
+            }
+
+            if (first is string firstString)
+            {
+                if (!(second is string secondString))
+                {
+                    if (!TryConvertTo(second, culture, out secondString))
+                    {
+                        return false;
+                    }
+                }
+
+                result = culture.CompareInfo.Compare(firstString, secondString, ignoreCase ? CompareOptions.IgnoreCase : CompareOptions.None);
+                return true;
+            }
+
+            Type firstType = first.GetType();
+            Type secondType = second.GetType();
+            int firstIndex = TypeTableIndex(firstType);
+            int secondIndex = TypeTableIndex(secondType);
+            if (firstIndex != -1 && secondIndex != -1)
+            {
+                result = NumericCompare(first, second, firstIndex, secondIndex);
+                return true;
+            }
+
+            if (!TryConvertTo(second, firstType, culture, out object secondConverted))
+            {
+                return false;
+            }
+
+            if (first is IComparable firstComparable)
+            {
+                result = firstComparable.CompareTo(secondConverted);
+                return true;
+            }
+
+            if (first.Equals(second))
+            {
+                result = 0;
+                return true;
+            }
+
+            // At this point, we know that they aren't equal but we have no way of
+            // knowing which should compare greater than the other so we return false.
+            return false;
+        }
+
         /// <summary>
         /// Returns true if the language considers obj to be true
         /// </summary>

test/powershell/engine/Api/LanguagePrimitive.Tests.ps1

@@ -29,7 +29,7 @@ Describe "Language Primitive Tests" -Tags "CI" {
     }
 
     It "Casting recursive array to bool should not cause crash" {
-        $a[0] = $a = [PSObject](,1)
+        $a[0] = $a = [PSObject](, 1)
         [System.Management.Automation.LanguagePrimitives]::IsTrue($a) | Should -BeTrue
     }
 
@@ -46,4 +46,60 @@ Describe "Language Primitive Tests" -Tags "CI" {
         [System.Management.Automation.LanguagePrimitives]::GetEnumerable($dt) | ForEach-Object { $count++ }
         $count | Should -Be 2
     }
+
+    It "TryCompare should succeed on int and string" {
+        $result = $null
+        [System.Management.Automation.LanguagePrimitives]::TryCompare(1, "1", [ref] $result) | Should -BeTrue
+        $result | Should -Be 0
+    }
+
+    It "TryCompare should fail on int and datetime" {
+        $result = $null
+        [System.Management.Automation.LanguagePrimitives]::TryCompare(1, [datetime]::Now, [ref] $result) | Should -BeFalse
+    }
+
+    It "TryCompare should succeed on int and int and compare correctly smaller" {
+        $result = $null
+        [System.Management.Automation.LanguagePrimitives]::TryCompare(1, 2, [ref] $result) | Should -BeTrue
+        $result | Should -BeExactly -1
+    }
+
+    It "TryCompare should succeed on string and string and compare correctly greater" {
+        $result = $null
+        [System.Management.Automation.LanguagePrimitives]::TryCompare("bbb", "aaa", [ref] $result) | Should -BeTrue
+        $result | Should -BeExactly 1
+    }
+
+    It "TryCompare should succeed on string and string and compare case insensitive correctly" {
+        $result = $null
+        [System.Management.Automation.LanguagePrimitives]::TryCompare("AAA", "aaa", $true, [ref] $result) | Should -BeTrue
+        $result | Should -BeExactly 0
+    }
+
+    It "TryCompare with cultureInfo is culture sensitive" {
+        $result = $null
+        $swedish = [cultureinfo] 'sv-SE'
+        # in Swedish, åäö appears at the end of the alphabet, and should compare greater than o
+        $val = [System.Management.Automation.LanguagePrimitives]::TryCompare("ooo", "ååå", $false, $swedish, [ref] $result)
+        $val | Should -BeTrue
+        $result | Should -BeExactly -1
+    }
+
+    It "TryCompare compares greater than null as Compare" {
+        $result = $null
+
+        $compareResult = [System.Management.Automation.LanguagePrimitives]::Compare($null, 10)
+        $val = [System.Management.Automation.LanguagePrimitives]::TryCompare($null, 10, [ref] $result)
+        $val | Should -BeTrue
+        $result | Should -BeExactly $compareResult
+    }
+
+    It "TryCompare compares less than null as Compare" {
+        $result = $null
+
+        $compareResult = [System.Management.Automation.LanguagePrimitives]::Compare(10, $null)
+        $val = [System.Management.Automation.LanguagePrimitives]::TryCompare(10, $null, [ref] $result)
+        $val | Should -BeTrue
+        $result | Should -BeExactly $compareResult
+    }
 }