Skip to content

Optimize IntMap.Bin #995

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 27 commits into from
Mar 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
554f143
IntMap: Merge prefix and mask in Bin
meooow25 Mar 9, 2024
512eb9b
Update lookupGE benchmarks
meooow25 Mar 10, 2024
a016498
Tweaks
meooow25 Mar 10, 2024
a7069d4
Use bitwise or instead of addition for clarity
meooow25 Mar 10, 2024
ea86844
Tweaks and docs
meooow25 Mar 10, 2024
58cbc6a
A slightly more efficient nomatch
meooow25 Mar 11, 2024
4b708ba
More efficient map-map branching
meooow25 Mar 12, 2024
82f4852
Remove getPrefix and getMask
meooow25 Mar 12, 2024
91c7a9e
Update prefix validation
meooow25 Mar 13, 2024
237178a
Tweaks and docs
meooow25 Mar 13, 2024
9ce5cc6
Simplify the validity check
meooow25 Mar 14, 2024
761fbda
Tweak invariant documentation
meooow25 Mar 16, 2024
a7da708
Add note on the relation to the Okasaki-Gill paper
meooow25 Mar 16, 2024
4ff6757
Use efficient hasPrefix in IntMapValidity
meooow25 Mar 27, 2024
afa9d61
Update Intmap/Prefix doc
meooow25 Mar 27, 2024
5e6505d
Document MapMapBranch
meooow25 Mar 27, 2024
b163af6
Test hasPrefix
meooow25 Mar 28, 2024
122a1d1
Add some notes for mapMapBranch
meooow25 Mar 28, 2024
e568f76
Tweak
meooow25 Mar 28, 2024
8fb07d0
Adopt BurningWitness's lower/upper helpers
meooow25 Mar 29, 2024
01de53a
Use bitwise or instead of plus for clarity (again)
meooow25 Mar 29, 2024
5922956
Keep this comment
meooow25 Mar 29, 2024
b59fe5f
Doc tweak
meooow25 Mar 29, 2024
8e1288b
Add property tests for is{Proper}SubmapOfBy
meooow25 Mar 29, 2024
5289272
Generate large keys in Arbitrary IntMap for better coverage
meooow25 Mar 29, 2024
4a9cc76
Add GHC flags for more stable benchmarks
meooow25 Mar 30, 2024
7e2ca1c
Remove -with-rtsopts=-A32m
meooow25 Mar 30, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 14 additions & 14 deletions containers-tests/benchmarks/LookupGE/LookupGE_IntMap.hs
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,18 @@ lookupGE1 k m =

lookupGE2 :: Key -> IntMap a -> Maybe (Key,a)
lookupGE2 k t = case t of
Bin _ m l r | m < 0 -> if k >= 0
Bin p l r | signBranch p -> if k >= 0
then go l
else case go r of
Nothing -> Just $ findMin l
justx -> justx
_ -> go t
where
go (Bin p m l r)
| nomatch k p m = if k < p
go (Bin p l r)
| nomatch k p = if k < unPrefix p
then Just $ findMin l
else Nothing
| zero k m = case go l of
| left k p = case go l of
Nothing -> Just $ findMin r
justx -> justx
| otherwise = go r
Expand All @@ -34,14 +34,14 @@ lookupGE2 k t = case t of

lookupGE3 :: Key -> IntMap a -> Maybe (Key,a)
lookupGE3 k t = k `seq` case t of
Bin _ m l r | m < 0 -> if k >= 0
Bin p l r | signBranch p -> if k >= 0
then go Nothing l
else go (Just (findMin l)) r
_ -> go Nothing t
where
go def (Bin p m l r)
| nomatch k p m = if k < p then Just $ findMin l else def
| zero k m = go (Just $ findMin r) l
go def (Bin p l r)
| nomatch k p = if k < unPrefix p then Just $ findMin l else def
| left k p = go (Just $ findMin r) l
| otherwise = go def r
go def (Tip ky y)
| k > ky = def
Expand All @@ -50,13 +50,13 @@ lookupGE3 k t = k `seq` case t of

lookupGE4 :: Key -> IntMap a -> Maybe (Key,a)
lookupGE4 k t = k `seq` case t of
Bin _ m l r | m < 0 -> if k >= 0 then go Nil l
else go l r
Bin p l r | signBranch p -> if k >= 0 then go Nil l
else go l r
_ -> go Nil t
where
go def (Bin p m l r)
| nomatch k p m = if k < p then fMin l else fMin def
| zero k m = go r l
go def (Bin p l r)
| nomatch k p = if k < unPrefix p then fMin l else fMin def
| left k p = go r l
| otherwise = go def r
go def (Tip ky y)
| k > ky = fMin def
Expand All @@ -66,7 +66,7 @@ lookupGE4 k t = k `seq` case t of
fMin :: IntMap a -> Maybe (Key, a)
fMin Nil = Nothing
fMin (Tip ky y) = Just (ky, y)
fMin (Bin _ _ l _) = fMin l
fMin (Bin _ l _) = fMin l

-------------------------------------------------------------------------------
-- Utilities
Expand Down
8 changes: 8 additions & 0 deletions containers-tests/containers-tests.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ common benchmark-deps
, deepseq >=1.1.0.0 && <1.6
, tasty-bench >=0.3.1 && <0.4

-- Flags recommended by tasty-bench
if impl(ghc >= 8.6)
ghc-options: -fproc-alignment=64

-- Copy of containers library,
library
import: deps
Expand All @@ -70,7 +74,11 @@ library

include-dirs: include
hs-source-dirs: src, tests

ghc-options: -O2 -Wall
if impl(ghc >= 8.6)
ghc-options: -fproc-alignment=64

other-extensions:
BangPatterns
CPP
Expand Down
86 changes: 49 additions & 37 deletions containers-tests/tests/IntMapValidity.hs
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
module IntMapValidity (valid) where
module IntMapValidity
( valid
, hasPrefix
, hasPrefixSimple
) where

import Data.Bits (xor, (.&.))
import Data.Bits (finiteBitSize, testBit, xor, (.&.))
import Data.List (intercalate, elemIndex)
import Data.IntMap.Internal
import Numeric (showHex)
import Test.Tasty.QuickCheck (Property, counterexample, property, (.&&.))
import Utils.Containers.Internal.BitUtil (bitcount)

{--------------------------------------------------------------------
Assertions
Expand All @@ -12,54 +17,61 @@ import Utils.Containers.Internal.BitUtil (bitcount)
valid :: IntMap a -> Property
valid t =
counterexample "nilNeverChildOfBin" (nilNeverChildOfBin t) .&&.
counterexample "commonPrefix" (commonPrefix t) .&&.
counterexample "maskRespected" (maskRespected t)
counterexample "prefixOk" (prefixOk t)

-- Invariant: Nil is never found as a child of Bin.
nilNeverChildOfBin :: IntMap a -> Bool
nilNeverChildOfBin t =
case t of
Nil -> True
Tip _ _ -> True
Bin _ _ l r -> noNilInSet l && noNilInSet r
Bin _ l r -> noNilInSet l && noNilInSet r
where
noNilInSet t' =
case t' of
Nil -> False
Tip _ _ -> True
Bin _ _ l' r' -> noNilInSet l' && noNilInSet r'
Bin _ l' r' -> noNilInSet l' && noNilInSet r'

-- Invariant: The Mask is a power of 2. It is the largest bit position at which
-- two keys of the map differ.
maskPowerOfTwo :: IntMap a -> Bool
maskPowerOfTwo t =
-- Invariants:
-- * All keys in a Bin start with the Bin's shared prefix.
-- * All keys in the Bin's left child have the Prefix's mask bit unset.
-- * All keys in the Bin's right child have the Prefix's mask bit set.
prefixOk :: IntMap a -> Property
prefixOk t =
case t of
Nil -> True
Tip _ _ -> True
Bin _ m l r ->
bitcount 0 (fromIntegral m) == 1 && maskPowerOfTwo l && maskPowerOfTwo r
Nil -> property ()
Tip _ _ -> property ()
Bin p l r ->
let px = unPrefix p
m = px .&. (-px)
keysl = keys l
keysr = keys r
debugStr = concat
[ "px=" ++ showIntHex px
, ", keysl=[" ++ intercalate "," (fmap showIntHex keysl) ++ "]"
, ", keysr=[" ++ intercalate "," (fmap showIntHex keysr) ++ "]"
]
in counterexample debugStr $
counterexample "mask bit absent" (px /= 0) .&&.
counterexample "prefix not shared" (all (`hasPrefix` p) (keysl ++ keysr)) .&&.
counterexample "left child, mask found set" (all (\x -> x .&. m == 0) keysl) .&&.
counterexample "right child, mask found unset" (all (\x -> x .&. m /= 0) keysr)

-- Invariant: Prefix is the common high-order bits that all elements share to
-- the left of the Mask bit.
commonPrefix :: IntMap a -> Bool
commonPrefix t =
case t of
Nil -> True
Tip _ _ -> True
b@(Bin p _ l r) -> all (sharedPrefix p) (keys b) && commonPrefix l && commonPrefix r
hasPrefix :: Int -> Prefix -> Bool
hasPrefix i p = not (nomatch i p)

-- We test that hasPrefix behaves the same as hasPrefixSimple.
hasPrefixSimple :: Int -> Prefix -> Bool
hasPrefixSimple k p = case elemIndex True pbits of
Nothing -> error "no mask bit" -- should not happen
Just i -> drop (i+1) kbits == drop (i+1) pbits
where
sharedPrefix :: Prefix -> Int -> Bool
sharedPrefix p a = p == p .&. a
kbits = toBits k
pbits = toBits (unPrefix p)

-- Invariant: In Bin prefix mask left right, left consists of the elements that
-- don't have the mask bit set; right is all the elements that do.
maskRespected :: IntMap a -> Bool
maskRespected t =
case t of
Nil -> True
Tip _ _ -> True
Bin _ binMask l r ->
all (\x -> zero x binMask) (keys l) &&
all (\x -> not (zero x binMask)) (keys r) &&
maskRespected l &&
maskRespected r
-- Bits from lowest to highest.
toBits x = fmap (testBit x) [0 .. finiteBitSize (0 :: Int) - 1]

showIntHex :: Int -> String
showIntHex x = "0x" ++ showHex (fromIntegral x :: Word) ""
43 changes: 40 additions & 3 deletions containers-tests/tests/intmap-properties.hs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ import Data.IntMap.Internal (traverseMaybeWithKey)
import Data.IntMap.Merge.Lazy
#endif
import Data.IntMap.Internal.Debug (showTree)
import IntMapValidity (valid)
import Data.IntMap.Internal (Prefix(..))
import IntMapValidity (hasPrefix, hasPrefixSimple, valid)

import Control.Applicative (Applicative(..))
import Control.Monad ((<=<))
Expand Down Expand Up @@ -134,6 +135,7 @@ main = defaultMain $ testGroup "intmap-properties"
, testCase "minimum" test_minimum
, testCase "maximum" test_maximum
, testProperty "valid" prop_valid
, testProperty "hasPrefix" prop_hasPrefix
, testProperty "empty valid" prop_emptyValid
, testProperty "insert to singleton" prop_singleton
, testProperty "insert then lookup" prop_insertLookup
Expand Down Expand Up @@ -209,6 +211,8 @@ main = defaultMain $ testGroup "intmap-properties"
, testProperty "traverseMaybeWithKey identity" prop_traverseMaybeWithKey_identity
, testProperty "traverseMaybeWithKey->mapMaybeWithKey" prop_traverseMaybeWithKey_degrade_to_mapMaybeWithKey
, testProperty "traverseMaybeWithKey->traverseWithKey" prop_traverseMaybeWithKey_degrade_to_traverseWithKey
, testProperty "isProperSubmapOfBy" prop_isProperSubmapOfBy
, testProperty "isSubmapOfBy" prop_isSubmapOfBy
]

apply2 :: Fun (a, b) c -> a -> b -> c
Expand All @@ -223,12 +227,23 @@ apply3 f a b c = apply f (a, b, c)
--------------------------------------------------------------------}

instance Arbitrary a => Arbitrary (IntMap a) where
arbitrary = fmap fromList arbitrary
arbitrary = oneof [go arbitrary, go (getLarge <$> arbitrary)]
where
go kgen = fromList <$> listOf ((,) <$> kgen <*> arbitrary)
shrink = fmap fromList . shrink . toAscList

newtype NonEmptyIntMap a = NonEmptyIntMap {getNonEmptyIntMap :: IntMap a} deriving (Eq, Show)

instance Arbitrary a => Arbitrary (NonEmptyIntMap a) where
arbitrary = fmap (NonEmptyIntMap . fromList . getNonEmpty) arbitrary
arbitrary = oneof [go arbitrary, go (getLarge <$> arbitrary)]
where
go kgen = NonEmptyIntMap . fromList <$> listOf1 ((,) <$> kgen <*> arbitrary)
shrink =
fmap (NonEmptyIntMap . fromList) .
List.filter (not . List.null) .
shrink .
toAscList .
getNonEmptyIntMap


------------------------------------------------------------------------
Expand Down Expand Up @@ -1150,6 +1165,10 @@ forValidUnitTree f = forValid f
prop_valid :: Property
prop_valid = forValidUnitTree $ \t -> valid t

prop_hasPrefix :: Int -> NonZero Int -> Property
prop_hasPrefix i (NonZero p) =
hasPrefix i (Prefix p) === hasPrefixSimple i (Prefix p)

----------------------------------------------------------------
-- QuickCheck
----------------------------------------------------------------
Expand Down Expand Up @@ -1641,3 +1660,21 @@ prop_traverseMaybeWithKey_degrade_to_traverseWithKey fun mp =
-- so this also checks the order of traversing is the same.
where f k v = (show k, applyFun2 fun k v)
g k v = fmap Just $ f k v

prop_isProperSubmapOfBy :: Fun (A, A) Bool -> IntMap A -> IntMap A -> Property
prop_isProperSubmapOfBy f m1 m2 =
isProperSubmapOfBy (applyFun2 f) m1 m2 ===
(length xs == size m1 && size m1 < size m2)
where
xs = List.intersectBy
(\(k1,x1) (k2,x2) -> k1 == k2 && applyFun2 f x1 x2)
(assocs m1) (assocs m2)

prop_isSubmapOfBy :: Fun (A, A) Bool -> IntMap A -> IntMap A -> Property
prop_isSubmapOfBy f m1 m2 =
isSubmapOfBy (applyFun2 f) m1 m2 ===
(length xs == size m1)
where
xs = List.intersectBy
(\(k1,x1) (k2,x2) -> k1 == k2 && applyFun2 f x1 x2)
(assocs m1) (assocs m2)
Loading