-
-
Notifications
You must be signed in to change notification settings - Fork 1k
Compare parameters by object value rather than string representation (Related to #2838) #2887
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
base: master
Are you sure you want to change the base?
Changes from all commits
6a58dae
a090785
5c265ae
24086de
ce085a1
f9562a3
4921e1b
7c651b7
21003f6
ab14463
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,9 @@ | ||
| using System; | ||
| using System.Collections; | ||
| using System.Collections.Generic; | ||
| using System.Linq; | ||
| using System.Reflection; | ||
| using BenchmarkDotNet.Portability; | ||
|
|
||
| namespace BenchmarkDotNet.Parameters | ||
| { | ||
|
|
@@ -18,30 +22,194 @@ public int Compare(ParameterInstances x, ParameterInstances y) | |
| if (compareTo != 0) | ||
| return compareTo; | ||
| } | ||
|
|
||
| return string.CompareOrdinal(x.DisplayInfo, y.DisplayInfo); | ||
| } | ||
|
|
||
| private int CompareValues(object x, object y) | ||
| private int CompareValues<T1, T2>(T1 x, T2 y) | ||
| { | ||
| // Detect IComparable implementations. | ||
| // This works for all primitive types in addition to user types that implement IComparable. | ||
| if (x != null && y != null && x.GetType() == y.GetType() && | ||
| x is IComparable xComparable) | ||
| if (x != null && y != null && x.GetType() == y.GetType()) | ||
| { | ||
| try | ||
| if (x is IStructuralComparable xStructuralComparable) | ||
| { | ||
| return xComparable.CompareTo(y); | ||
| if (x is Array xArr && y is Array yArr) | ||
| { | ||
| if (xArr.Rank != yArr.Rank) return xArr.Rank.CompareTo(yArr.Rank); | ||
|
|
||
| for (int dim = 0; dim < xArr.Rank; dim++) | ||
| { | ||
| if (xArr.GetLength(dim) != yArr.GetLength(dim)) | ||
| return xArr.GetLength(dim).CompareTo(yArr.GetLength(dim)); | ||
| } | ||
|
|
||
| // 1D, 2D, and 3D array comparison is optimized with dedicated methods | ||
| if (xArr.Rank == 1) return StructuralComparisonWithFallback(xArr, yArr); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need the fallback case for all structural comparisons, not just arrays (md arrays can be nested in the type). In fact, we can simplify it by just wrapping the normal case in a try/catch, and do the specialized array checks in the catch clause.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The other three cases (Rank 2, Rank 3, and Rank 4+) never fail because they recurse back into CompareValues, and the comparability stuff is handled there, so I only used the fallback for 1d arrays. As for other structural comparisons (arrays), I am unable to add a similar fallback because the element structure of the type would be unknown. In my new commit, it would just catch the error and fall to the default case at the bottom of the method
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right I understand those don't fail in the current code. But the else clause could fail for the same reason. else // Probably a user-defined IStructuralComparable, as tuples would be handled by the IComparable casee.g.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As for structural fallbacks, you can check if the type is array (which you already do), and also check for |
||
|
|
||
| if (xArr.Rank == 2 && !RuntimeInformation.IsAot) | ||
| { | ||
| return (int) GetType() | ||
| .GetMethod(nameof(CompareTwoDimensionalArray), BindingFlags.NonPublic | BindingFlags.Instance) | ||
| .MakeGenericMethod(xArr.GetType().GetElementType(), yArr.GetType().GetElementType()) | ||
| .Invoke(this, [xArr, yArr]); | ||
User0332 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| if (xArr.Rank == 3 && !RuntimeInformation.IsAot) | ||
| { | ||
| return (int) GetType() | ||
| .GetMethod(nameof(CompareThreeDimensionalArray), BindingFlags.NonPublic | BindingFlags.Instance) | ||
| .MakeGenericMethod(xArr.GetType().GetElementType(), yArr.GetType().GetElementType()) | ||
| .Invoke(this, [xArr, yArr]); | ||
| } | ||
|
|
||
| return CompareEnumerables(xArr, yArr); | ||
| } | ||
| else // Probably a user-defined IStructuralComparable or tuple | ||
| { | ||
| return StructuralComparisonWithFallback(xStructuralComparable, (IStructuralComparable) y); | ||
| } | ||
| } | ||
| else if (x is IComparable xComparable) | ||
| { | ||
| try | ||
| { | ||
| return xComparable.CompareTo(y); | ||
| } | ||
| // Some types, such as Tuple and ValueTuple, have a fallible CompareTo implementation which can throw if the inner items don't implement IComparable. | ||
| // See: https://github.com/dotnet/BenchmarkDotNet/issues/2346 | ||
| // For now, catch and ignore the exception, and fallback to string comparison below. | ||
| catch (ArgumentException ex) when (ex.Message.Contains("At least one object must implement IComparable.")) | ||
| { | ||
| } | ||
| } | ||
| // Some types, such as Tuple and ValueTuple, have a fallible CompareTo implementation which can throw if the inner items don't implement IComparable. | ||
| // See: https://github.com/dotnet/BenchmarkDotNet/issues/2346 | ||
| // For now, catch and ignore the exception, and fallback to string comparison below. | ||
| catch (ArgumentException ex) when (ex.Message.Contains("At least one object must implement IComparable.")) | ||
| else if (x is IEnumerable xEnumerable && y is IEnumerable yEnumerable) // General collection comparison support | ||
| { | ||
| return CompareEnumerables(xEnumerable, yEnumerable); | ||
| } | ||
| } | ||
|
|
||
| // Anything else. | ||
| // Anything else to differentiate between objects. | ||
| return string.CompareOrdinal(x?.ToString(), y?.ToString()); | ||
| } | ||
|
|
||
| private int StructuralComparisonWithFallback(IStructuralComparable x, IStructuralComparable y) | ||
| { | ||
| try | ||
| { | ||
| return StructuralComparisons.StructuralComparer.Compare(x, y); | ||
| } | ||
| // Some types, such as Tuple and ValueTuple, have a fallible CompareTo implementation which can throw if the inner items don't implement IComparable. | ||
| // See: https://github.com/dotnet/BenchmarkDotNet/issues/2346 | ||
| // For now, catch and ignore the exception, and fallback to string comparison below. | ||
| catch (ArgumentException ex) when (ex.Message.Contains("At least one object must implement IComparable.")) | ||
| { | ||
| var ITuple = Type.GetType("System.Runtime.CompilerServices.ITuple"); | ||
|
|
||
| if (ITuple.IsAssignableFrom(x.GetType())) | ||
| { | ||
| var lengthProperty = ITuple.GetProperty("Length"); | ||
|
|
||
| var xLength = (int) lengthProperty.GetValue(x); | ||
| var yLength = (int) lengthProperty.GetValue(y); | ||
|
|
||
| if (xLength != yLength) return xLength.CompareTo(yLength); | ||
|
|
||
| var indexerProperty = ITuple.GetProperty("Item"); | ||
|
|
||
| object ItemGetter(IStructuralComparable tup, int i) => indexerProperty.GetValue(tup, [i]); | ||
|
|
||
| var xFlatTransformed = Enumerable.Range(0, xLength).Select(i => ItemGetter(x, i).ToString()); | ||
| var yFlatTransformed = Enumerable.Range(0, xLength).Select(i => ItemGetter(y, i).ToString()); | ||
|
|
||
| return CompareEnumerables(xFlatTransformed, yFlatTransformed); | ||
| } | ||
| else | ||
| { | ||
| return string.CompareOrdinal(x?.ToString(), y?.ToString()); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private int StructuralComparisonWithFallback(Array x, Array y) | ||
| { | ||
| try | ||
| { | ||
| return StructuralComparisons.StructuralComparer.Compare(x, y); | ||
| } | ||
| // Inner element type does not support comparison, ToString (GetHashCode not preferred) elements to compare collections | ||
| catch (ArgumentException ex) when (ex.Message.Contains("At least one object must implement IComparable.")) | ||
| { | ||
| var xFlatTransformed = x.OfType<object>().Select(elem => elem.ToString()); | ||
| var yFlatTransformed = y.OfType<object>().Select(elem => elem.ToString()); | ||
| return CompareEnumerables(xFlatTransformed, yFlatTransformed); | ||
| } | ||
| } | ||
|
|
||
| private int CompareEnumerables(IEnumerable x, IEnumerable y) | ||
| { | ||
| // Use this instead of StructuralComparisons.StructuralComparer to avoid resolving the whole enumerable to object[] | ||
|
|
||
| var xEnumer = x.GetEnumerator(); | ||
| var yEnumer = y.GetEnumerator(); | ||
|
|
||
| bool xHasElement, yHasElement; | ||
|
|
||
| // Use bitwise AND to avoid short-circuiting, which destroys this function's length checking logic | ||
| while ((xHasElement = xEnumer.MoveNext()) & (yHasElement = yEnumer.MoveNext())) | ||
| { | ||
| int res = CompareValues(xEnumer.Current, yEnumer.Current); | ||
|
|
||
| if (res != 0) return res; | ||
| } | ||
|
|
||
| if (xHasElement) return 1; | ||
| if (yHasElement) return -1; | ||
|
|
||
| return 0; | ||
| } | ||
|
|
||
| private int CompareTwoDimensionalArray<T1, T2>(T1[,] arrOne, T2[,] arrTwo) | ||
| { | ||
| // Assume that arrOne & arrTwo are the same Length & Rank | ||
|
|
||
| for (int i = 0; i < arrOne.GetLength(0); i++) | ||
| { | ||
| for (int j = 0; j < arrOne.GetLength(1); j++) | ||
| { | ||
| var x = arrOne[i, j]; | ||
| var y = arrTwo[i, j]; | ||
|
|
||
| var res = CompareValues(x, y); | ||
|
|
||
| if (res != 0) return res; | ||
| } | ||
| } | ||
|
|
||
| return 0; | ||
| } | ||
|
|
||
| private int CompareThreeDimensionalArray<T1, T2>(T1[,,] arrOne, T2[,,] arrTwo) | ||
| { | ||
| // Assume that arrOne & arrTwo are the same Length & Rank | ||
|
|
||
| for (int i = 0; i < arrOne.GetLength(0); i++) | ||
| { | ||
| for (int j = 0; j < arrOne.GetLength(1); j++) | ||
| { | ||
| for (int k = 0; k <arrOne.GetLength(2); k++) | ||
| { | ||
| var x = arrOne[i, j, k]; | ||
| var y = arrTwo[i, j, k]; | ||
|
|
||
| var res = CompareValues(x, y); | ||
|
|
||
| if (res != 0) return res; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return 0; | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,162 @@ | ||
| using System; | ||
| using System.Collections; | ||
| using System.Collections.Generic; | ||
| using System.Linq; | ||
| using System.Reflection; | ||
| using BenchmarkDotNet.Portability; | ||
|
|
||
| namespace BenchmarkDotNet.Parameters | ||
| { | ||
| internal class ParameterEqualityComparer : IEqualityComparer<ParameterInstances> | ||
| { | ||
| public static readonly ParameterEqualityComparer Instance = new ParameterEqualityComparer(); | ||
|
|
||
| public bool Equals(ParameterInstances x, ParameterInstances y) | ||
| { | ||
| if (x == null && y == null) return true; | ||
| if (x != null && y == null) return false; | ||
| if (x == null) return false; | ||
|
|
||
| for (int i = 0; i < Math.Min(x.Count, y.Count); i++) | ||
| { | ||
| var isEqual = ValuesEqual(x[i]?.Value, y[i]?.Value); | ||
|
|
||
| if (!isEqual) return false; | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| private bool ValuesEqual<T1, T2>(T1 x, T2 y) | ||
| { | ||
| if (x == null && y == null) return true; | ||
|
|
||
| if (x != null && y != null && x.GetType() == y.GetType()) | ||
| { | ||
| if (x is IStructuralEquatable xStructuralEquatable) | ||
| { | ||
| if (x is Array xArr && y is Array yArr) | ||
| { | ||
| if (xArr.Rank != yArr.Rank) return false; | ||
|
|
||
| for (int dim = 0; dim < xArr.Rank; dim++) | ||
| { | ||
| if (xArr.GetLength(dim) != yArr.GetLength(dim)) return false; | ||
| } | ||
|
|
||
| // 1D, 2D, and 3D array comparison is optimized with dedicated methods | ||
| if (xArr.Rank == 1) return StructuralEquals(xArr, yArr); | ||
|
|
||
| if (xArr.Rank == 2 && !RuntimeInformation.IsAot) | ||
| { | ||
| return (bool) GetType() | ||
| .GetMethod(nameof(TwoDimensionalArrayEquals), BindingFlags.NonPublic | BindingFlags.Instance) | ||
| .MakeGenericMethod(xArr.GetType().GetElementType(), yArr.GetType().GetElementType()) | ||
| .Invoke(this, [xArr, yArr]); | ||
| } | ||
|
|
||
| if (xArr.Rank == 3 && !RuntimeInformation.IsAot) | ||
| { | ||
| return (bool) GetType() | ||
| .GetMethod(nameof(ThreeDimensionalArrayEquals), BindingFlags.NonPublic | BindingFlags.Instance) | ||
| .MakeGenericMethod(xArr.GetType().GetElementType(), yArr.GetType().GetElementType()) | ||
| .Invoke(this, [xArr, yArr]); | ||
| } | ||
|
|
||
| return EnumerablesEqual(xArr, yArr); | ||
| } | ||
| else // Probably a user-defined IStructuralEquatable or tuple | ||
| { | ||
| return StructuralEquals(xStructuralEquatable, (IStructuralEquatable) y); | ||
| } | ||
| } | ||
| else if (x is IEnumerable xEnumerable && y is IEnumerable yEnumerable) // General collection equality support | ||
| { | ||
| return EnumerablesEqual(xEnumerable, yEnumerable); | ||
| } | ||
| else | ||
| { | ||
| return x.Equals(y); | ||
| } | ||
| } | ||
|
|
||
| // The objects are of different types or one is null, they cannot be equal | ||
| return false; | ||
| } | ||
|
|
||
| private bool StructuralEquals(IStructuralEquatable x, IStructuralEquatable y) | ||
| { | ||
| return StructuralComparisons.StructuralEqualityComparer.Equals(x, y); | ||
| } | ||
|
|
||
| private bool EnumerablesEqual(IEnumerable x, IEnumerable y) | ||
| { | ||
| // Use this instead of StructuralComparisons.StructuralComparer to avoid resolving the whole enumerable to object[] | ||
|
|
||
| var xEnumer = x.GetEnumerator(); | ||
| var yEnumer = y.GetEnumerator(); | ||
|
|
||
| bool xHasElement, yHasElement; | ||
|
|
||
| // Use bitwise AND to avoid short-circuiting, which destroys this function's length checking logic | ||
| while ((xHasElement = xEnumer.MoveNext()) & (yHasElement = yEnumer.MoveNext())) | ||
| { | ||
| bool res = ValuesEqual(xEnumer.Current, yEnumer.Current); | ||
|
|
||
| if (!res) return false; | ||
| } | ||
|
|
||
| if (xHasElement || yHasElement) return false; | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| private bool TwoDimensionalArrayEquals<T1, T2>(T1[,] arrOne, T2[,] arrTwo) | ||
| { | ||
| // Assume that arrOne & arrTwo are the same Length & Rank | ||
|
|
||
| for (int i = 0; i < arrOne.GetLength(0); i++) | ||
| { | ||
| for (int j = 0; j < arrOne.GetLength(1); j++) | ||
| { | ||
| var x = arrOne[i, j]; | ||
| var y = arrTwo[i, j]; | ||
|
|
||
| bool res = ValuesEqual(x, y); | ||
|
|
||
| if (!res) return false; | ||
| } | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| private bool ThreeDimensionalArrayEquals<T1, T2>(T1[,,] arrOne, T2[,,] arrTwo) | ||
| { | ||
| // Assume that arrOne & arrTwo are the same Length & Rank | ||
|
|
||
| for (int i = 0; i < arrOne.GetLength(0); i++) | ||
| { | ||
| for (int j = 0; j < arrOne.GetLength(1); j++) | ||
| { | ||
| for (int k = 0; k <arrOne.GetLength(2); k++) | ||
| { | ||
| var x = arrOne[i, j, k]; | ||
| var y = arrTwo[i, j, k]; | ||
|
|
||
| bool res = ValuesEqual(x, y); | ||
|
|
||
| if (!res) return false; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| public int GetHashCode(ParameterInstances obj) | ||
| { | ||
| return obj?.ValueInfo.GetHashCode() ?? 0; | ||
| } | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.