From 05e9fbf8eacfc487f57d5bbd81e66ae38d9db9cd Mon Sep 17 00:00:00 2001 From: Simon Jakobi Date: Fri, 5 Dec 2025 08:05:13 +0100 Subject: [PATCH 1/5] WIP: Refactor `delete` --- Data/HashMap/Internal.hs | 47 ++++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/Data/HashMap/Internal.hs b/Data/HashMap/Internal.hs index 924f221f..adbb8ef9 100644 --- a/Data/HashMap/Internal.hs +++ b/Data/HashMap/Internal.hs @@ -1158,26 +1158,27 @@ unsafeInsertWithKey f k0 v0 m0 = runST (go h0 k0 v0 0 m0) -- | \(O(\log n)\) Remove the mapping for the specified key from this map -- if present. delete :: Hashable k => k -> HashMap k v -> HashMap k v -delete k m = delete' (hash k) k m -{-# INLINABLE delete #-} +delete k = delete' (hash k) k +{-# INLINE delete #-} delete' :: Eq k => Hash -> k -> HashMap k v -> HashMap k v -delete' h0 k0 m0 = deleteFromSubtree h0 k0 0 m0 -{-# INLINABLE delete' #-} +delete' = deleteFromSubtree 0 +{-# INLINE delete' #-} -- | This version of 'delete' can be used on subtrees when a the -- corresponding 'Shift' argument is supplied. -deleteFromSubtree :: Eq k => Hash -> k -> Shift -> HashMap k v -> HashMap k v -deleteFromSubtree !_ !_ !_ Empty = Empty -deleteFromSubtree h k _ t@(Leaf hy (L ky _)) - | hy == h && ky == k = Empty - | otherwise = t -deleteFromSubtree h k s t@(BitmapIndexed b ary) - | b .&. m == 0 = t - | otherwise - = case A.index# ary i of +deleteFromSubtree :: Eq k => Shift -> Hash -> k -> HashMap k v -> HashMap k v +deleteFromSubtree !s !h !k = \case + Empty -> Empty + t@(Leaf hy (L ky _)) + | hy == h && ky == k -> Empty + | otherwise -> t + t@(BitmapIndexed b ary) + | b .&. m == 0 -> t + | otherwise -> + case A.index# ary i of (# !st #) -> - case deleteFromSubtree h k (nextShift s) st of + case deleteFromSubtree (nextShift s) h k st of Empty | A.length ary == 2 , (# l #) <- A.index# ary (otherOfOneOrZero i) , isLeafOrCollision l @@ -1187,27 +1188,27 @@ deleteFromSubtree h k s t@(BitmapIndexed b ary) st' | isLeafOrCollision st' && A.length ary == 1 -> st' | st' `ptrEq` st -> t | otherwise -> BitmapIndexed b (A.update ary i st') - where m = mask h s - i = sparseIndex b m -deleteFromSubtree h k s t@(Full ary) = + where m = mask h s + i = sparseIndex b m + t@(Full ary) -> case A.index# ary i of (# !st #) -> - case deleteFromSubtree h k (nextShift s) st of + case deleteFromSubtree (nextShift s) h k st of Empty -> let ary' = A.delete ary i bm = fullBitmap .&. complement (1 `unsafeShiftL` i) in BitmapIndexed bm ary' st' | st' `ptrEq` st -> t | otherwise -> Full (updateFullArray ary i st') - where i = index h s -deleteFromSubtree h k _ t@(Collision hy v) + where i = index h s + t@(Collision hy v) | h == hy , Just i <- indexOf k v - = if A.length v == 2 + -> if A.length v == 2 then case A.index# v (otherOfOneOrZero i) of (# l #) -> Leaf h l else Collision h (A.delete v i) - | otherwise = t + | otherwise -> t {-# INLINABLE deleteFromSubtree #-} -- | Delete optimized for the case when we know the key is in the map. @@ -1845,7 +1846,7 @@ difference = go_difference 0 go_difference s t1@(Leaf h1 (L k1 _)) t2 = lookupCont (\_ -> t1) (\_ _ -> Empty) h1 k1 s t2 go_difference _ t1 Empty = t1 - go_difference s t1 (Leaf h2 (L k2 _)) = deleteFromSubtree h2 k2 s t1 + go_difference s t1 (Leaf h2 (L k2 _)) = deleteFromSubtree s h2 k2 t1 go_difference s t1@(BitmapIndexed b1 ary1) (BitmapIndexed b2 ary2) = differenceArrays s b1 ary1 t1 b2 ary2 From e1544057e436201b079e7a828d7950605e0ac009 Mon Sep 17 00:00:00 2001 From: Simon Jakobi Date: Fri, 5 Dec 2025 08:22:13 +0100 Subject: [PATCH 2/5] Move ptr-eq check within st' alternative --- Data/HashMap/Internal.hs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Data/HashMap/Internal.hs b/Data/HashMap/Internal.hs index adbb8ef9..8ba24b65 100644 --- a/Data/HashMap/Internal.hs +++ b/Data/HashMap/Internal.hs @@ -1185,8 +1185,8 @@ deleteFromSubtree !s !h !k = \case -> l | otherwise -> BitmapIndexed (b .&. complement m) (A.delete ary i) - st' | isLeafOrCollision st' && A.length ary == 1 -> st' - | st' `ptrEq` st -> t + st' | st' `ptrEq` st -> t + | isLeafOrCollision st' && A.length ary == 1 -> st' | otherwise -> BitmapIndexed b (A.update ary i st') where m = mask h s i = sparseIndex b m From ec9b9c42eaf18c91a645d96a9d13295fb2b4cc34 Mon Sep 17 00:00:00 2001 From: Simon Jakobi Date: Fri, 5 Dec 2025 08:31:41 +0100 Subject: [PATCH 3/5] Formatting --- Data/HashMap/Internal.hs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/Data/HashMap/Internal.hs b/Data/HashMap/Internal.hs index 8ba24b65..4c3ba041 100644 --- a/Data/HashMap/Internal.hs +++ b/Data/HashMap/Internal.hs @@ -1175,8 +1175,7 @@ deleteFromSubtree !s !h !k = \case | otherwise -> t t@(BitmapIndexed b ary) | b .&. m == 0 -> t - | otherwise -> - case A.index# ary i of + | otherwise -> case A.index# ary i of (# !st #) -> case deleteFromSubtree (nextShift s) h k st of Empty | A.length ary == 2 @@ -1204,10 +1203,10 @@ deleteFromSubtree !s !h !k = \case t@(Collision hy v) | h == hy , Just i <- indexOf k v - -> if A.length v == 2 - then case A.index# v (otherOfOneOrZero i) of - (# l #) -> Leaf h l - else Collision h (A.delete v i) + -> if A.length v == 2 + then case A.index# v (otherOfOneOrZero i) of + (# l #) -> Leaf h l + else Collision h (A.delete v i) | otherwise -> t {-# INLINABLE deleteFromSubtree #-} From 817814426647d1d2e744866a48b7d818bdf67a22 Mon Sep 17 00:00:00 2001 From: Simon Jakobi Date: Fri, 5 Dec 2025 08:32:43 +0100 Subject: [PATCH 4/5] Improve comment --- Data/HashMap/Internal.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Data/HashMap/Internal.hs b/Data/HashMap/Internal.hs index 4c3ba041..3f73be33 100644 --- a/Data/HashMap/Internal.hs +++ b/Data/HashMap/Internal.hs @@ -1165,7 +1165,7 @@ delete' :: Eq k => Hash -> k -> HashMap k v -> HashMap k v delete' = deleteFromSubtree 0 {-# INLINE delete' #-} --- | This version of 'delete' can be used on subtrees when a the +-- | This version of 'delete' can be used on a subtree when the -- corresponding 'Shift' argument is supplied. deleteFromSubtree :: Eq k => Shift -> Hash -> k -> HashMap k v -> HashMap k v deleteFromSubtree !s !h !k = \case From 5f77e92f32c7a658cf02a4a7f7d79b9abf2bcb15 Mon Sep 17 00:00:00 2001 From: Simon Jakobi Date: Fri, 5 Dec 2025 08:35:04 +0100 Subject: [PATCH 5/5] Improve variable naming --- Data/HashMap/Internal.hs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Data/HashMap/Internal.hs b/Data/HashMap/Internal.hs index 3f73be33..2156ab57 100644 --- a/Data/HashMap/Internal.hs +++ b/Data/HashMap/Internal.hs @@ -1200,13 +1200,13 @@ deleteFromSubtree !s !h !k = \case st' | st' `ptrEq` st -> t | otherwise -> Full (updateFullArray ary i st') where i = index h s - t@(Collision hy v) + t@(Collision hy ary) | h == hy - , Just i <- indexOf k v - -> if A.length v == 2 - then case A.index# v (otherOfOneOrZero i) of + , Just i <- indexOf k ary + -> if A.length ary == 2 + then case A.index# ary (otherOfOneOrZero i) of (# l #) -> Leaf h l - else Collision h (A.delete v i) + else Collision h (A.delete ary i) | otherwise -> t {-# INLINABLE deleteFromSubtree #-}