Intern Qualified, use HashMaps for type checking environment by kozak · Pull Request #9 · restaumatic/purescript

kozak · 2025-05-27T11:15:12Z

Cleaned up.

stat A: min 16ms max 17ms mean 16ms median 16ms stddev 0ms n=18
stat B: min 13ms max 14ms mean 13ms median 13ms stddev 0ms n=18
mean diff -20.6%
ttest detected diff? true p-value 0.000000
stat ratios: min -23.50% max -16.32% mean -20.61% median -20.69% stddev 2.28% n=18
ttest detected ratios diff? true p-value 0.000000

…-interning

zyla · 2025-05-27T12:08:34Z

src/Language/PureScript/Interner.hs

+{-# LANGUAGE ViewPatterns #-}
+{-# LANGUAGE ScopedTypeVariables #-}
+
+module Language.PureScript.Interner where


It's taken from https://github.com/reflex-frp/hash-cons/blob/main/src/Data/HashCons/Internal.hs , right?
Are there modifications? Maybe add a comment indicating the source?
Also, the license probably should be copied here or something

We definitely should

zyla · 2025-05-27T12:13:18Z

src/Language/PureScript/Names.hs

  | UnusedIdent
  -- |
-  -- A generated name used only for internal transformations
+  -- A generated name used only for hashConsal transformations


A clbuttic mistake

Suggested change

-- A generated name used only for hashConsal transformations

-- A generated name used only for internal transformations

zyla · 2025-05-27T12:15:59Z

src/Language/PureScript/Names.hs

+instance Show ModuleName where
+  show (ModuleName i) = T.unpack $ unHashCons i
+
+instance Ord ModuleName where
+  compare (ModuleName a) (ModuleName b) = compare (unHashCons a) (unHashCons b)
+
+instance Serialise ModuleName where
+  encode (ModuleName i) = encode (unHashCons i)
+  decode = ModuleName . hashCons <$> decode
+
+instance Hashable ModuleName where
+  hash (ModuleName i) = hash i
+  hashWithSalt s (ModuleName i) = hashWithSalt s i


These instances can probably be handled via newtype deriving.

zyla · 2025-05-27T12:16:44Z

src/Language/PureScript/Names.hs

+  show (ModuleName i) = T.unpack $ unHashCons i
+
+instance Ord ModuleName where
+  compare (ModuleName a) (ModuleName b) = compare (unHashCons a) (unHashCons b)


Is there a reason we don't use the "optimized" hash-based Ord? Do we rely on deterministic ordering somewhere?

zyla · 2025-05-27T12:18:41Z

src/Language/PureScript/Names.hs

 --
-data Qualified a = Qualified QualifiedBy a
-  deriving (Show, Eq, Ord, Functor, Foldable, Traversable, Generic)
+data Qualified' a = Qualified' QualifiedBy a


I don't know what the convention is in Haskell, but in Rust-land such types ("guts" of a type where the public version is wrapped in something) are named by adding an Inner suffix, i.e. QualifiedInner

zyla · 2025-05-27T12:22:11Z

src/Language/PureScript/Names.hs


-instance NFData a => NFData (Qualified a)
-instance Serialise a => Serialise (Qualified a)
+mapQualified :: Hashable b => (a -> b) -> Qualified a -> Qualified b


It is a little annoying (lots of changes caused by losing the Functor instance).

If only the Haskell ecosystem used constrained functors by default...

zyla · 2025-05-27T12:24:08Z

src/Language/PureScript/Names.hs

    byModule = do
      (mn, a) <- parseJSON2 v
-      pure $ Qualified (ByModuleName mn) a
+      pure $ mkQualified_ (ByModuleName mn) a


Can we just use the pattern synonym here?

Suggested change

pure $ mkQualified_ (ByModuleName mn) a

pure $ Qualified (ByModuleName mn) a

(and in a lot of other places)

Weird because I thought I have already done that, maybe it was a different branch..

zyla · 2025-05-27T12:26:13Z

src/Language/PureScript/PSString.hs

+newtype PSString = PSString { unPSString :: [Word16] }
+  deriving (Eq, NFData, Generic)
+  deriving newtype Hashable

-instance NFData PSString
-instance Serialise PSString
+instance Ord PSString where
+  compare (PSString a) (PSString b) = compare a b

 instance Show PSString where
  show = show . codePoints

+toUTF16CodeUnits :: PSString -> [Word16]
+toUTF16CodeUnits (PSString ps) = ps
+
+mkPSString :: [Word16] -> PSString
+mkPSString = PSString
+
+
+instance Semigroup PSString where
+  PSString a <> PSString b = PSString (a <> b)
+
+instance Monoid PSString where
+  mempty = PSString []
+  mappend = (<>)
+
+instance Codec.Serialise PSString where
+  encode (PSString s) = Codec.encode s
+  decode = mkPSString <$> Codec.decode
+
+


I assume these changes are preparation for also interning PSString?

You are correct, this is a leftover from interning. I reverted it but didn't revert the changes.

zyla · 2025-05-27T12:29:04Z

src/Language/PureScript/Names.hs

+  decode = ProperName . hashCons <$> decode
+
+instance Ord (ProperName a) where
+  compare (ProperName a) (ProperName b) = compare (unHashCons a) (unHashCons b)


Same with ModuleName, why can't we use HashCons' Ord?

kozak added 16 commits May 9, 2025 22:24

Module name

42b7e6a

Add proper name interning

ab7348a

generalise intern

16fa18b

PSString interning

8889163

Qualified by hash

71eafab

Hash map for typeclass dictionaries

89b04a9

Hash Type a and use a hash set in unification cache

261465d

Merge branch 'various-perf-changes' into hash-type

ff661e5

Merge branch 'interning-qualified-by-hash' into hash-type-with-interning

a7751b5

WIP Add hashable

ba88b9d

Compiles but is super slow

cf755a7

Hash cons

ea7bb65

Hash maps

51486e5

Int map for unification

f69b497

Optimize occurs check

7bc51cd

Use Hashed and HashSet instead of IntMap to address hash conflicts

9550a4c

kozak requested review from jborkowski and zyla May 27, 2025 11:15

kozak changed the title ~~hash type with interning~~ Intern QualifiedBy use HashMaps for type checking environment May 27, 2025

kozak changed the title ~~Intern QualifiedBy use HashMaps for type checking environment~~ Intern Qualified, use HashMaps for type checking environment May 27, 2025

kozak added 6 commits May 27, 2025 11:51

Cleanup warnings

a090ddb

Fix type search ordering and tests

2773298

Lint error

e86c6c1

Lint error

1a183bd

Remove perf.txt

9d098fb

Merge remote-tracking branch 'origin/restaumatic' into hash-type-with…

955210d

…-interning

zyla reviewed Jun 3, 2025

View reviewed changes

	-- A generated name used only for hashConsal transformations
	-- A generated name used only for internal transformations

	pure $ mkQualified_ (ByModuleName mn) a
	pure $ Qualified (ByModuleName mn) a

Conversation

kozak commented May 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

kozak commented May 27, 2025 •

edited

Loading