Refactor delete
#571
Refactor delete
#571
Conversation
| st' | st' `ptrEq` st -> t | ||
| | isLeafOrCollision st' && A.length ary == 1 -> st' |
There was a problem hiding this comment.
This change slightly reduces the code size by moving the pointer-eq check to an earlier point. Only afterwards do we differentiate the alternatives for Leaf, Collision etc.
| | otherwise | ||
| = case A.index# ary i of | ||
| deleteFromSubtree :: Eq k => Shift -> Hash -> k -> HashMap k v -> HashMap k v | ||
| deleteFromSubtree !s !h !k = \case |
There was a problem hiding this comment.
I'm a bit unhappy about the forcing of keys. The upside is that key types likes Int and ShortByteString are unboxed. The downside is that key types that can't be unboxed to an unlifted representation are unnecessarily evaluated again and again.
Maybe we can use Strict# for this, once it's released?!
There was a problem hiding this comment.
Where/why are keys getting re-forced?
There was a problem hiding this comment.
They are forced once for each subtree we check. So probably ~1–5 times for a typical HashMap.
There was a problem hiding this comment.
You're only forcing the key to get it to unbox, right? So you need the function to be strict in the key when the key is an unboxable type. The function is "naturally" strict in the key for (non-trivial, non-pathological) unboxable types whenever its hash is present in the map (because you'll test key equality). So I believe you should only need to add explicit strictness in the key where you discover that its hash is not in the map.
There was a problem hiding this comment.
Interesting! I agree with your reasoning but I doubt that GHC's strictness analyser could be convinced to believe it too! 😄
There was a problem hiding this comment.
I bet it can, but I need to go back to sleep.
No description provided.