-
Notifications
You must be signed in to change notification settings - Fork 178
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
Fusible IntSet.fromDistinctAscList definition #951
Comments
I tweaked the implementation to store masks on the stack instead of prefixes, saving some calculations since the mask is what is compared every time in With it, the new timings are GHC 9.2.5, new
GHC 9.6.2, new
Now it always takes less time vs current on 9.6.2, even for The implementationfromDistinctAscList :: [Key] -> IntSet
fromDistinctAscList = finish . Foldable.foldl' next StateEmpty
where
next st x = case st of
StateEmpty -> State px bx Nada
State py by stk
| px == py -> State px (by .|. bx) stk
| otherwise -> State px bx (linkTop (branchMask px py) py (Tip py by) stk)
where
px = prefixOf x
bx = bitmapOf x
finish StateEmpty = Nil
finish (State pr br stk) = linkAll pr (Tip pr br) stk
{-# INLINE fromDistinctAscList #-}
linkTop :: Mask -> Prefix -> IntSet -> Stack -> Stack
linkTop !m !pr !tr (Push mlr tl stk)
| shorter m mlr = linkTop m pr (Bin (mask pr mlr) mlr tl tr) stk
linkTop m _ tl stk = Push m tl stk
linkAll :: Prefix -> IntSet -> Stack -> IntSet
linkAll !_ !tr Nada = tr
linkAll pr tr (Push mlr tl stk)
| mlr < 0 = Bin 0 mlr tr tl
| otherwise = linkAll pr (Bin (mask pr mlr) mlr tl tr) stk
data FromDistinctMonoState = State {-# UNPACK #-} !Prefix {-# UNPACK #-} !BitMap !Stack | StateEmpty
data Stack = Push {-# UNPACK #-} !Mask !IntSet !Stack | Nada |
Do you want to open a PR? |
I do, but I would prefer to be done with the similar Set/Map stuff before getting to this properly. |
Sister issue to #949. I adapted the old algorithm from #658 into a fold to see if it improves things for IntSet like it did for Set.
Benchmarking 4 scenarios
GHC 9.2.5, current
GHC 9.2.5, new
GHC 9.6.2, current
GHC 9.6.2, new
As expected
However, it is worse in time for
fromDistinctAscList:sparse
, which is disappointing.I'll try to figure out if it can be improved. Happy to hear if you have any ideas for that.
If it does not improve, I think this is still not a bad idea. The improvement on
fromDistinctAscList:dense:fusion
looks great. We could use rewrite rules to try to fuse and fall back to the current implementation, as we considered in #949.The text was updated successfully, but these errors were encountered: