From 46c967060f3ac9a3f3f58c2733f6ad68a538165e Mon Sep 17 00:00:00 2001 From: Orson Peters Date: Tue, 10 Oct 2023 22:32:26 +0200 Subject: [PATCH 1/4] Optimize ord implementation --- src/lib.rs | 42 +++++++++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 15 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index d17ce5a..a90bc0b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -116,25 +116,37 @@ impl PartialOrd for OrderedFloat { fn partial_cmp(&self, other: &Self) -> Option { Some(self.cmp(other)) } + + fn lt(&self, other: &Self) -> bool { + !(self >= other) + } + + fn le(&self, other: &Self) -> bool { + other >= self + } + + fn gt(&self, other: &Self) -> bool { + !(other >= self) + } + + fn ge(&self, other: &Self) -> bool { + // We consider all NaNs equal, and NaN is the largest possible + // value. Thus if self is NaN we always return true. Otherwise + // self >= other is correct. If other is also not NaN it is trivially + // correct, and if it is we note that nothing can be greater or + // equal to NaN except NaN itself, which we already handled earlier. + self.0.is_nan() | (self.0 >= other.0) + } } impl Ord for OrderedFloat { fn cmp(&self, other: &Self) -> Ordering { - let lhs = &self.0; - let rhs = &other.0; - match lhs.partial_cmp(rhs) { - Some(ordering) => ordering, - None => { - if lhs.is_nan() { - if rhs.is_nan() { - Ordering::Equal - } else { - Ordering::Greater - } - } else { - Ordering::Less - } - } + if self < other { + Ordering::Less + } else if self > other { + Ordering::Greater + } else { + Ordering::Equal } } } From deff8d08fcf8d5dce29a653c56062405b7c58274 Mon Sep 17 00:00:00 2001 From: Orson Peters Date: Tue, 10 Oct 2023 22:44:50 +0200 Subject: [PATCH 2/4] Add exhaustive infinite/finite/zero/nan combination test --- tests/test.rs | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/test.rs b/tests/test.rs index 69dfd6d..752ecbf 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -21,6 +21,31 @@ fn not_nan(x: T) -> NotNan { NotNan::new(x).unwrap() } +#[test] +fn test_total_order() { + let numberline = [ + (-f32::INFINITY, 0), + (-1.0, 1), + (-0.0, 2), (0.0, 2), + (1.0, 3), + (f32::INFINITY, 4), + (f32::NAN, 5), + (-f32::NAN, 5), + ]; + + for &(fi, i) in &numberline { + for &(fj, j) in &numberline { + assert_eq!(OrderedFloat(fi) < OrderedFloat(fj), i < j); + assert_eq!(OrderedFloat(fi) > OrderedFloat(fj), i > j); + assert_eq!(OrderedFloat(fi) <= OrderedFloat(fj), i <= j); + assert_eq!(OrderedFloat(fi) >= OrderedFloat(fj), i >= j); + assert_eq!(OrderedFloat(fi) == OrderedFloat(fj), i == j); + assert_eq!(OrderedFloat(fi) != OrderedFloat(fj), i != j); + assert_eq!(OrderedFloat(fi).cmp(&OrderedFloat(fj)), i.cmp(&j)); + } + } +} + #[test] fn ordered_f32_compare_regular_floats() { assert_eq!(OrderedFloat(7.0f32).cmp(&OrderedFloat(7.0)), Equal); From 06884aed7412aaa5f93478981cf7ee6f17a1ec1a Mon Sep 17 00:00:00 2001 From: Orson Peters Date: Tue, 10 Oct 2023 22:54:32 +0200 Subject: [PATCH 3/4] Optimize signed zero canonicalization --- src/lib.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index a90bc0b..5afa427 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -41,7 +41,14 @@ const MAN_MASK: u64 = 0x000fffffffffffffu64; // canonical raw bit patterns (for hashing) const CANONICAL_NAN_BITS: u64 = 0x7ff8000000000000u64; -const CANONICAL_ZERO_BITS: u64 = 0x0u64; + +#[inline(always)] +fn canonicalize_signed_zero(x: T) -> T { + // -0.0 + 0.0 == +0.0 under IEEE754 roundTiesToEven rounding mode, + // which Rust guarantees. Thus by adding a positive zero we + // canonicalize signed zero without any branches in one instruction. + x + T::zero() +} /// A wrapper around floats providing implementations of `Eq`, `Ord`, and `Hash`. /// @@ -173,10 +180,8 @@ impl Hash for OrderedFloat { fn hash(&self, state: &mut H) { let bits = if self.is_nan() { CANONICAL_NAN_BITS - } else if self.is_zero() { - CANONICAL_ZERO_BITS } else { - raw_double_bits(&self.0) + raw_double_bits(&canonicalize_signed_zero(self.0)) }; bits.hash(state) @@ -1162,12 +1167,7 @@ impl Ord for NotNan { impl Hash for NotNan { #[inline] fn hash(&self, state: &mut H) { - let bits = if self.is_zero() { - CANONICAL_ZERO_BITS - } else { - raw_double_bits(&self.0) - }; - + let bits = raw_double_bits(&canonicalize_signed_zero(self.0)); bits.hash(state) } } From 71770b32728362f9b9482ee0f46ee6441abdc1c9 Mon Sep 17 00:00:00 2001 From: Orson Peters Date: Tue, 10 Oct 2023 22:57:27 +0200 Subject: [PATCH 4/4] Silence clippy, fmt, and add missing inline directives --- src/lib.rs | 12 +++++++++--- tests/test.rs | 5 +++-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 5afa427..5a512e7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -124,18 +124,22 @@ impl PartialOrd for OrderedFloat { Some(self.cmp(other)) } + #[inline] fn lt(&self, other: &Self) -> bool { - !(self >= other) + !self.ge(other) } + #[inline] fn le(&self, other: &Self) -> bool { - other >= self + other.ge(self) } + #[inline] fn gt(&self, other: &Self) -> bool { - !(other >= self) + !other.ge(self) } + #[inline] fn ge(&self, other: &Self) -> bool { // We consider all NaNs equal, and NaN is the largest possible // value. Thus if self is NaN we always return true. Otherwise @@ -147,7 +151,9 @@ impl PartialOrd for OrderedFloat { } impl Ord for OrderedFloat { + #[inline] fn cmp(&self, other: &Self) -> Ordering { + #[allow(clippy::comparison_chain)] if self < other { Ordering::Less } else if self > other { diff --git a/tests/test.rs b/tests/test.rs index 752ecbf..5626d1c 100644 --- a/tests/test.rs +++ b/tests/test.rs @@ -26,13 +26,14 @@ fn test_total_order() { let numberline = [ (-f32::INFINITY, 0), (-1.0, 1), - (-0.0, 2), (0.0, 2), + (-0.0, 2), + (0.0, 2), (1.0, 3), (f32::INFINITY, 4), (f32::NAN, 5), (-f32::NAN, 5), ]; - + for &(fi, i) in &numberline { for &(fj, j) in &numberline { assert_eq!(OrderedFloat(fi) < OrderedFloat(fj), i < j);