From de3a6b828ed93949378e9c435672f5d17362b50a Mon Sep 17 00:00:00 2001 From: Yoh Deadfall Date: Fri, 19 May 2023 20:46:26 +0300 Subject: [PATCH 1/3] Optimized metrics names equality comparison `SetEqual` is available since .NET Fx 3.5 and is destined for equality comparison of two different hash set instances. Since both instances of the `Metrics` type has the same default comparer, the method works as a sugar over a length check followed by iteration over one set and probing the second one for containing the current element through the `Contains` method. --- .../Core/Infer/Metric/Metrics.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/Elastic.Clients.Elasticsearch/Core/Infer/Metric/Metrics.cs b/src/Elastic.Clients.Elasticsearch/Core/Infer/Metric/Metrics.cs index 808bc73c7cd..973fd275670 100644 --- a/src/Elastic.Clients.Elasticsearch/Core/Infer/Metric/Metrics.cs +++ b/src/Elastic.Clients.Elasticsearch/Core/Infer/Metric/Metrics.cs @@ -4,7 +4,6 @@ using System; using System.Collections.Generic; -using System.Linq; using Elastic.Transport; namespace Elastic.Clients.Elasticsearch; @@ -53,8 +52,8 @@ public bool Equals(Metrics other) { if (other is null) return false; - // Equality is true when the metrics names in both instances are equal, regardless of their order in the set. - return Values.OrderBy(t => t).SequenceEqual(other.Values.OrderBy(t => t)); + // Equality is true when both instances have the same metric names. + return Values.SetEquals(other.Values); } string IUrlParameter.GetString(ITransportConfiguration settings) => GetString(); From d91e2ec38da7a593b083292d15bddf9073d7278e Mon Sep 17 00:00:00 2001 From: Yoh Deadfall Date: Fri, 19 May 2023 21:05:18 +0300 Subject: [PATCH 2/3] Fixed GetHashCode implementation on Metrics type `HashSet` doesn't override `GetHashCode`. Therefore, the default provided algorithm for reference types is used which doesn't take fields into account as it happens for value types. That breaks the equality contracts on Metrics. Ideally `HashCode` type should be used for more correct computations with less number of collisions, but that requires increasing the minimal target framework. --- .../Core/Infer/Metric/Metrics.cs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/Elastic.Clients.Elasticsearch/Core/Infer/Metric/Metrics.cs b/src/Elastic.Clients.Elasticsearch/Core/Infer/Metric/Metrics.cs index 973fd275670..cea6dbd6ab3 100644 --- a/src/Elastic.Clients.Elasticsearch/Core/Infer/Metric/Metrics.cs +++ b/src/Elastic.Clients.Elasticsearch/Core/Infer/Metric/Metrics.cs @@ -70,7 +70,16 @@ private string GetString() } /// - public override int GetHashCode() => Values != null ? Values.GetHashCode() : 0; + public override int GetHashCode() + { + // Lifting the minimal target framework to .NET Standard 2.1 + // would be the best solution ever due to the HashCode type. + var hashCode = 0; + foreach (var metric in Values) + hashCode ^= metric.GetHashCode(); + + return hashCode; + } public static bool operator ==(Metrics left, Metrics right) => Equals(left, right); public static bool operator !=(Metrics left, Metrics right) => !Equals(left, right); From 3cf87fc1290bd3fa377159b84c7bbdc6c640b843 Mon Sep 17 00:00:00 2001 From: Yoh Deadfall Date: Mon, 22 May 2023 20:05:59 +0300 Subject: [PATCH 3/3] Improved hash code generation --- src/Elastic.Clients.Elasticsearch/Core/Infer/Metric/Metrics.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Elastic.Clients.Elasticsearch/Core/Infer/Metric/Metrics.cs b/src/Elastic.Clients.Elasticsearch/Core/Infer/Metric/Metrics.cs index cea6dbd6ab3..68cc21015f3 100644 --- a/src/Elastic.Clients.Elasticsearch/Core/Infer/Metric/Metrics.cs +++ b/src/Elastic.Clients.Elasticsearch/Core/Infer/Metric/Metrics.cs @@ -76,7 +76,7 @@ public override int GetHashCode() // would be the best solution ever due to the HashCode type. var hashCode = 0; foreach (var metric in Values) - hashCode ^= metric.GetHashCode(); + hashCode = (hashCode * 397) ^ metric.GetHashCode(); return hashCode; }