Skip to content

Conversation

@sjakobi
Copy link
Member

@sjakobi sjakobi commented Nov 26, 2025

Eq has been a superclass of Hashable since hashable-1.4, which is the minimum version u-c supports.

`Eq` has been a superclass of `Hashable` since `hashable-1.4`, which is
the minimum version u-c supports.
@sjakobi sjakobi force-pushed the sjakobi/redundant-eq branch from dfa6414 to 83d1130 Compare November 27, 2025 12:48
@treeowl
Copy link
Collaborator

treeowl commented Nov 27, 2025

The only downside is that the performance will be even worse if specialization fails. I think we always assume it succeeds, though, so that shouldn't be a big deal. Best to check benchmarks anyway.

@sjakobi
Copy link
Member Author

sjakobi commented Nov 27, 2025

The only downside is that the performance will be even worse if specialization fails.

Can you explain why?

Best to check benchmarks anyway.

Do you have a suggestion how to benchmark this? Just add -fno-specialise?!

@treeowl
Copy link
Collaborator

treeowl commented Nov 27, 2025

I just meant to check that the normal benchmarks will be okay, and that optimization doesn't get thrown off. Now that I think about it some more, I'm not actually sure if things will be worse in case of specialization failure, since the Hashable dictionary will need to be loaded into cache anyway. Never mind.

@sjakobi
Copy link
Member Author

sjakobi commented Nov 28, 2025

So this does change things a bit more than I expected. E.g.

--- RHS size: {terms: 22, types: 17, coercions: 0, joins: 0/0}
+-- RHS size: {terms: 22, types: 16, coercions: 0, joins: 0/0}
 insert [InlPrag=INLINABLE]
-  :: forall k v.
-     (Eq k, Hashable k) =>
-     k -> v -> HashMap k v -> HashMap k v
+  :: forall k v. Hashable k => k -> v -> HashMap k v -> HashMap k v
 [GblId,
- Arity=5,
- Str=<SP(SC(S,C(1,L)),A)><SP(A,A,1C(1,L))><SL><1L><1L>,
+ Arity=4,
+ Str=<SP(SP(SC(S,C(1,L)),A),A,1C(1,L))><SL><1L><1L>,
  Unf=Unf{Src=StableUser, TopLvl=True,
          Value=True, ConLike=True, WorkFree=True, Expandable=True,
-         Guidance=IF_ARGS [0 30 0 20 0] 120 0}]
+         Guidance=IF_ARGS [60 0 20 0] 140 0}]
 insert
   = \ (@k)
       (@v)
-      ($dEq :: Eq k)
       ($dHashable :: Hashable k)
       (k1 :: k)
       (v1 :: v)
       (eta :: HashMap k v) ->
       case v1 of v2 { __DEFAULT ->
       case hash @k $dHashable k1 of { I# i ->
-      $winsert' @k @v $dEq (int2Word# i) k1 v2 eta
+      $winsert' @k @v ($p1Hashable @k $dHashable) (int2Word# i) k1 v2 eta
       }
       }

So if this ends up being used without specialization we pass one dict less, but then have to extract the Eq dict from $dHashable. I don't like that the Eq dict is allocated and passed as a thunk though. Do you know how this can be avoided?

In other cases it's purely beneficial:

--- RHS size: {terms: 16, types: 19, coercions: 0, joins: 0/0}
+-- RHS size: {terms: 14, types: 17, coercions: 0, joins: 0/0}
 update [InlPrag=INLINABLE]
   :: forall k a.
-     (Eq k, Hashable k) =>
+     Hashable k =>
      (a -> Maybe a) -> k -> HashMap k a -> HashMap k a
 [GblId,
- Arity=5,
- Str=<SP(LC(L,C(1,L)),A)><SP(A,A,SC(S,L))><MC(1,L)><SL><SL>,
+ Arity=4,
+ Str=<SP(SP(LC(L,C(1,L)),A),A,SC(S,L))><MC(1,L)><SL><SL>,
  Unf=Unf{Src=StableUser, TopLvl=True,
          Value=True, ConLike=True, WorkFree=True, Expandable=True,
-         Guidance=IF_ARGS [0 0 0 0 0] 100 0}]
+         Guidance=IF_ARGS [0 0 0 0] 90 0}]
 update
   = \ (@k)
       (@a)
-      ($dEq :: Eq k)
       ($dHashable :: Hashable k)
       (f :: a -> Maybe a)
       (eta :: k)
       (eta1 :: HashMap k a) ->
       alter
         @k
         @a
-        $dEq
         $dHashable
         (\ (v [OS=OneShot] :: Maybe a) -> $fMonadMaybe_$c>>= @a @a v f)
         eta
         eta1

@sjakobi
Copy link
Member Author

sjakobi commented Nov 29, 2025

I don't like that the Eq dict is allocated and passed as a thunk though. Do you know how this can be avoided?

Maybe -fspec-eval-dictfun would help. Will give it a try.

@sjakobi
Copy link
Member Author

sjakobi commented Nov 29, 2025

I don't like that the Eq dict is allocated and passed as a thunk though. Do you know how this can be avoided?

GHC 9.12.2 seems to take care of this somewhere between -ddump-simpl and -ddump-prep. At the Core prep stage, all the $p1Hashable @k $dHashable expressions are forced.

@sjakobi
Copy link
Member Author

sjakobi commented Nov 29, 2025

So for non-specialized uses of u-c this is possibly still a mixed bag, but I doubt it's a large effect one way or the other.

@sjakobi sjakobi merged commit cf586d7 into master Nov 29, 2025
10 checks passed
@sjakobi sjakobi deleted the sjakobi/redundant-eq branch November 29, 2025 06:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants