From 00a3c22972066aae44f94c677c67ccd35fe77c78 Mon Sep 17 00:00:00 2001 From: Dag Brattli Date: Wed, 29 Jul 2026 18:53:15 +0200 Subject: [PATCH 1/3] perf(python): hash numbers without the safeHash detour MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `identityHash` sent Decimal, BigInt, Int64 and UInt64 to `safeHash` while every other numeric width went to `numberHash` — another grouping that makes sense for JavaScript, where these are polyfilled objects, and not for Python. Both routes end at the same `GetHashCode`. `safeHash` just delegates to `identity_hash`, which re-checks the hashable protocol before making the same call, so the detour buys a frame and a check and returns an identical value: type same safeHash numberHash Int64 True 100.8 ns 74.2 ns UInt64 True 100.2 ns 76.6 ns Decimal True 243.5 ns 133.3 ns BigInt True 215.8 ns 103.2 ns Rather than move the two widths that prompted this, drop the special case: all four are equivalent, and `identityHash` now agrees with `structuralHash`, which has always sent every `Number` to `numberHash`. That is also what the comment above the match already claimed. Covered by new tests asserting GetHashCode agrees with `hash`, distinguishes distinct values, and still works for HashSet keys, across all four types. Co-Authored-By: Claude Opus 5 (1M context) --- src/Fable.Transforms/Python/Replacements.fs | 5 +++- tests/Python/TestComparison.fs | 32 +++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/Fable.Transforms/Python/Replacements.fs b/src/Fable.Transforms/Python/Replacements.fs index eeb2c4dfe6..997d42d43d 100644 --- a/src/Fable.Transforms/Python/Replacements.fs +++ b/src/Fable.Transforms/Python/Replacements.fs @@ -558,7 +558,10 @@ let identityHash com r (arg: Expr) = | Char | String | Builtin BclGuid -> "stringHash" - | Number((Decimal | BigInt | Int64 | UInt64), _) -> "safeHash" + // Every numeric width reaches the same `GetHashCode`, so routing the wider + // ones through `safeHash` only added a frame and its protocol check. This + // now matches `structuralHash`, which has always sent all of them to + // `numberHash`. | Number _ | Builtin BclTimeSpan -> "numberHash" | List _ -> "safeHash" diff --git a/tests/Python/TestComparison.fs b/tests/Python/TestComparison.fs index fbcdeb0d94..14df5eb1f0 100644 --- a/tests/Python/TestComparison.fs +++ b/tests/Python/TestComparison.fs @@ -656,3 +656,35 @@ let ``CompareTo with primitives works`` () = (1.).CompareTo(1.) |> equal 0 (1.m).CompareTo(1.m) |> equal 0 ("1").CompareTo("1") |> equal 0 + +[] +let ``test GetHashCode with wide numeric types works`` () = + // These route through identity hashing, which used to take a different path + // than structural hashing for the widths below. + (5L).GetHashCode() |> equal ((5L).GetHashCode()) + (5UL).GetHashCode() |> equal ((5UL).GetHashCode()) + (5M).GetHashCode() |> equal ((5M).GetHashCode()) + (5I).GetHashCode() |> equal ((5I).GetHashCode()) + (5L).GetHashCode() = (6L).GetHashCode() |> equal false + (5UL).GetHashCode() = (6UL).GetHashCode() |> equal false + (5M).GetHashCode() = (6M).GetHashCode() |> equal false + (5I).GetHashCode() = (6I).GetHashCode() |> equal false + +[] +let ``test hashing wide numeric types agrees with GetHashCode`` () = + hash 5L |> equal ((5L).GetHashCode()) + hash 5UL |> equal ((5UL).GetHashCode()) + hash 5M |> equal ((5M).GetHashCode()) + hash 5I |> equal ((5I).GetHashCode()) + +[] +let ``test wide numeric types work as hash keys`` () = + let longs = System.Collections.Generic.HashSet([ 1L; 2L; 3L ]) + longs.Contains 2L |> equal true + longs.Contains 9L |> equal false + let ulongs = System.Collections.Generic.HashSet([ 1UL; 2UL ]) + ulongs.Contains 2UL |> equal true + let decimals = System.Collections.Generic.HashSet([ 1M; 2M ]) + decimals.Contains 2M |> equal true + let bigs = System.Collections.Generic.HashSet([ 1I; 2I ]) + bigs.Contains 2I |> equal true From c96c5d489fd5e051cbac83d1149117883f364800 Mon Sep 17 00:00:00 2001 From: Dag Brattli Date: Wed, 29 Jul 2026 18:53:24 +0200 Subject: [PATCH 2/3] docs(python): correct the fill_typed_vec extraction-order rationale MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The comment said the wrapper type is tried first "so a same-type value costs no conversion dunder". That held when Int32 and Float64 values were wrapper objects; since they became plain ints and floats the wrapper arm always fails for exactly those two, so the sentence now describes the opposite of what happens on the common path. No behaviour change — the order is fine as it stands. A failed pyo3 `extract` is a Rust-level `Err`, not a constructed Python exception, and measures at 68.1 ns against 66.6 ns for the arm that hits. Co-Authored-By: Claude Opus 5 (1M context) --- src/fable-library-py/src/native_array.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/fable-library-py/src/native_array.rs b/src/fable-library-py/src/native_array.rs index 09d8810b30..0a09b0c52b 100644 --- a/src/fable-library-py/src/native_array.rs +++ b/src/fable-library-py/src/native_array.rs @@ -167,9 +167,11 @@ impl Clone for NativeArray { // Helper macro for filling storage // -// The wrapper type is tried first so a same-type value costs no conversion dunder; -// the primitive fallback accepts a plain Python int/float, which is how Int32 and -// Float64 values are represented. +// The wrapper type is tried first, which hits for the widths that are still pyo3 +// wrappers. Int32 and Float64 are plain Python ints and floats, so for those two +// the wrapper arm always fails and the primitive fallback is the normal path — +// which costs nothing measurable, because a failed `extract` is a Rust-level +// `Err` rather than a constructed Python exception. macro_rules! fill_typed_vec { ($vec:expr, $value:expr, $target_index:expr, $count:expr, $type:ty, $prim:ty) => {{ let typed_value: $prim = match $value.extract::<$type>() { From e20d8edc4b53f5a34d4188f7ae8c5a1c3e5f9707 Mon Sep 17 00:00:00 2001 From: Dag Brattli Date: Wed, 29 Jul 2026 19:07:58 +0200 Subject: [PATCH 3/3] chore(python): build the hash-key test sets without a sequence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Constructing a HashSet from an IEnumerable makes the generated code hand an `IEnumerable_1[T]` to Python's `set(...)`, which Pyright rejects because `IEnumerable_1` has no `__iter__`. That is the same pre-existing gap that keeps `test_hash_set.py` in the CI exclude list, and the new test added four fresh instances of it to a file that is not excluded. Adding the elements one at a time avoids it and tests the same thing — arguably more of it, since asserting that a duplicate `Add` returns false requires the hash and equality lookup to work rather than merely not crash. Co-Authored-By: Claude Opus 5 (1M context) --- tests/Python/TestComparison.fs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/tests/Python/TestComparison.fs b/tests/Python/TestComparison.fs index 14df5eb1f0..d8374f46be 100644 --- a/tests/Python/TestComparison.fs +++ b/tests/Python/TestComparison.fs @@ -679,12 +679,21 @@ let ``test hashing wide numeric types agrees with GetHashCode`` () = [] let ``test wide numeric types work as hash keys`` () = - let longs = System.Collections.Generic.HashSet([ 1L; 2L; 3L ]) + // Added one at a time rather than from a sequence: constructing a HashSet from + // an IEnumerable trips a known Pyright gap, the one that keeps test_hash_set.py + // in the CI exclude list. + let longs = System.Collections.Generic.HashSet() + longs.Add 1L |> ignore + longs.Add 2L |> ignore + longs.Add 2L |> equal false longs.Contains 2L |> equal true longs.Contains 9L |> equal false - let ulongs = System.Collections.Generic.HashSet([ 1UL; 2UL ]) + let ulongs = System.Collections.Generic.HashSet() + ulongs.Add 2UL |> ignore ulongs.Contains 2UL |> equal true - let decimals = System.Collections.Generic.HashSet([ 1M; 2M ]) + let decimals = System.Collections.Generic.HashSet() + decimals.Add 2M |> ignore decimals.Contains 2M |> equal true - let bigs = System.Collections.Generic.HashSet([ 1I; 2I ]) + let bigs = System.Collections.Generic.HashSet() + bigs.Add 2I |> ignore bigs.Contains 2I |> equal true