Skip to content

Commit dc3ce15

Browse files
treeowloliver-batchelor
authored andcommitted
Use unsafePerformIO better (#325)
The previous code was struggling to create artificial dependencies to prevent `unsafePerformIO` calls from being floated out of their proper context. Push the context into each `unsafePerformIO` call so the dependency is real. Reading a bit of the resulting Core, I don't expect to see many (if any) performance regressions. Performance will hopefully improve a bit if/when [GHC issue 15127](https://gitlab.haskell.org/ghc/ghc/issues/15127) is fixed.
1 parent 13e76bb commit dc3ce15

File tree

1 file changed

+51
-46
lines changed

1 file changed

+51
-46
lines changed

src/Reflex/Spider/Internal.hs

Lines changed: 51 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -198,12 +198,6 @@ nextNodeIdRef = unsafePerformIO $ newIORef 1
198198

199199
newNodeId :: IO Int
200200
newNodeId = atomicModifyIORef' nextNodeIdRef $ \n -> (succ n, n)
201-
202-
{-# NOINLINE unsafeNodeId #-}
203-
unsafeNodeId :: a -> Int
204-
unsafeNodeId a = unsafePerformIO $ do
205-
touch a
206-
newNodeId
207201
#endif
208202

209203
--------------------------------------------------------------------------------
@@ -307,9 +301,10 @@ cacheEvent e =
307301
#else
308302
Event $
309303
#endif
310-
let mSubscribedRef :: IORef (FastWeak (CacheSubscribed x a))
311-
!mSubscribedRef = unsafeNewIORef e emptyFastWeak
312-
in \sub -> {-# SCC "cacheEvent" #-} do
304+
unsafePerformIO $ do
305+
mSubscribedRef :: IORef (FastWeak (CacheSubscribed x a))
306+
<- newIORef emptyFastWeak
307+
pure $ \sub -> {-# SCC "cacheEvent" #-} do
313308
#ifdef DEBUG_TRACE_EVENTS
314309
unless (BS8.null callSite) $ liftIO $ BS8.hPutStrLn stderr callSite
315310
#endif
@@ -1177,18 +1172,12 @@ buildDynamic readV0 v' = do
11771172
return d
11781173

11791174
unsafeBuildDynamic :: BehaviorM x (PatchTarget p) -> Event x p -> Dyn x p
1180-
unsafeBuildDynamic readV0 v' = Dyn $ unsafeNewIORef x $ UnsafeDyn x
1181-
where x = (readV0, v')
1175+
unsafeBuildDynamic readV0 v' =
1176+
Dyn $ unsafePerformIO $ newIORef $ UnsafeDyn (readV0, v')
11821177

11831178
-- ResultM can read behaviors and events
11841179
type ResultM = EventM
11851180

1186-
{-# NOINLINE unsafeNewIORef #-}
1187-
unsafeNewIORef :: a -> b -> IORef b
1188-
unsafeNewIORef a b = unsafePerformIO $ do
1189-
touch a
1190-
newIORef b
1191-
11921181
instance HasSpiderTimeline x => Functor (Event x) where
11931182
fmap f = push $ return . Just . f
11941183

@@ -1201,26 +1190,35 @@ push f e = cacheEvent (pushCheap f e)
12011190

12021191
{-# INLINABLE pull #-}
12031192
pull :: BehaviorM x a -> Behavior x a
1204-
pull a = behaviorPull $ Pull
1205-
{ pullCompute = a
1206-
, pullValue = unsafeNewIORef a Nothing
1193+
pull a = unsafePerformIO $ do
1194+
ref <- newIORef Nothing
12071195
#ifdef DEBUG_NODEIDS
1208-
, pullNodeId = unsafeNodeId a
1196+
nid <- newNodeId
12091197
#endif
1210-
}
1198+
pure $ behaviorPull $ Pull
1199+
{ pullCompute = a
1200+
, pullValue = ref
1201+
#ifdef DEBUG_NODEIDS
1202+
, pullNodeId = nid
1203+
#endif
1204+
}
12111205

12121206
{-# INLINABLE switch #-}
12131207
switch :: HasSpiderTimeline x => Behavior x (Event x a) -> Event x a
1214-
switch a = eventSwitch $ Switch
1215-
{ switchParent = a
1216-
, switchSubscribed = unsafeNewIORef a Nothing
1217-
}
1208+
switch a = unsafePerformIO $ do
1209+
ref <- newIORef Nothing
1210+
pure $ eventSwitch $ Switch
1211+
{ switchParent = a
1212+
, switchSubscribed = ref
1213+
}
12181214

12191215
coincidence :: HasSpiderTimeline x => Event x (Event x a) -> Event x a
1220-
coincidence a = eventCoincidence $ Coincidence
1221-
{ coincidenceParent = a
1222-
, coincidenceSubscribed = unsafeNewIORef a Nothing
1223-
}
1216+
coincidence a = unsafePerformIO $ do
1217+
ref <- newIORef Nothing
1218+
pure $ eventCoincidence $ Coincidence
1219+
{ coincidenceParent = a
1220+
, coincidenceSubscribed = ref
1221+
}
12241222

12251223
-- Propagate the given event occurrence; before cleaning up, run the given action, which may read the state of events and behaviors
12261224
run :: forall x b. HasSpiderTimeline x => [DSum (RootTrigger x) Identity] -> ResultM x b -> SpiderHost x b
@@ -1424,6 +1422,9 @@ getRootSubscribed k r sub = do
14241422
when debugPropagate $ putStrLn $ "getRootSubscribed: calling rootInit"
14251423
uninit <- rootInit r k $ RootTrigger (subs, rootOccurrence r, k)
14261424
writeIORef uninitRef $! uninit
1425+
#ifdef DEBUG_NODEIDS
1426+
nid <- newNodeId
1427+
#endif
14271428
let !subscribed = RootSubscribed
14281429
{ rootSubscribedKey = k
14291430
, rootSubscribedCachedSubscribed = cached
@@ -1432,7 +1433,7 @@ getRootSubscribed k r sub = do
14321433
, rootSubscribedUninit = uninit
14331434
, rootSubscribedWeakSelf = weakSelf
14341435
#ifdef DEBUG_NODEIDS
1435-
, rootSubscribedNodeId = unsafeNodeId (k, r, subs)
1436+
, rootSubscribedNodeId = nid
14361437
#endif
14371438
}
14381439
-- If we die at the same moment that all our children die, they will
@@ -1481,16 +1482,10 @@ newFanInt = do
14811482
, _fanInt_occRef = occRef
14821483
}
14831484

1484-
{-# NOINLINE unsafeNewFanInt #-}
1485-
unsafeNewFanInt :: b -> FanInt x a
1486-
unsafeNewFanInt b = unsafePerformIO $ do
1487-
touch b
1488-
newFanInt
1489-
14901485
fanInt :: HasSpiderTimeline x => Event x (IntMap a) -> EventSelectorInt x a
1491-
fanInt p =
1492-
let self = unsafeNewFanInt p
1493-
in EventSelectorInt $ \k -> Event $ \sub -> do
1486+
fanInt p = unsafePerformIO $ do
1487+
self <- newFanInt
1488+
pure $ EventSelectorInt $ \k -> Event $ \sub -> do
14941489
isEmpty <- liftIO $ FastMutableIntMap.isEmpty (_fanInt_subscribers self)
14951490
when isEmpty $ do -- This is the first subscriber, so we need to subscribe to our input
14961491
(subscription, parentOcc) <- subscribeAndRead p $ Subscriber
@@ -1544,13 +1539,16 @@ getFanSubscribed k f sub = do
15441539
subscribersRef <- liftIO $ newIORef $ error "getFanSubscribed: subscribersRef not yet initialized"
15451540
occRef <- liftIO $ newIORef parentOcc
15461541
when (isJust parentOcc) $ scheduleClear occRef
1542+
#ifdef DEBUG_NODEIDS
1543+
nid <- liftIO newNodeId
1544+
#endif
15471545
let subscribed = FanSubscribed
15481546
{ fanSubscribedCachedSubscribed = fanSubscribed f
15491547
, fanSubscribedOccurrence = occRef
15501548
, fanSubscribedParent = subscription
15511549
, fanSubscribedSubscribers = subscribersRef
15521550
#ifdef DEBUG_NODEIDS
1553-
, fanSubscribedNodeId = unsafeNodeId f
1551+
, fanSubscribedNodeId = nid
15541552
#endif
15551553
}
15561554
let !self = (k, subscribed)
@@ -1610,6 +1608,9 @@ getSwitchSubscribed s sub = do
16101608
when (isJust parentOcc) $ scheduleClear occRef
16111609
weakSelf <- liftIO $ newIORef $ error "getSwitchSubscribed: weakSelf not yet initialized"
16121610
(subs, slnForSub) <- liftIO $ WeakBag.singleton sub weakSelf cleanupSwitchSubscribed
1611+
#ifdef DEBUG_NODEIDS
1612+
nid <- liftIO newNodeId
1613+
#endif
16131614
let !subscribed = SwitchSubscribed
16141615
{ switchSubscribedCachedSubscribed = switchSubscribed s
16151616
, switchSubscribedOccurrence = occRef
@@ -1622,7 +1623,7 @@ getSwitchSubscribed s sub = do
16221623
, switchSubscribedCurrentParent = subscriptionRef
16231624
, switchSubscribedWeakSelf = weakSelf
16241625
#ifdef DEBUG_NODEIDS
1625-
, switchSubscribedNodeId = unsafeNodeId s
1626+
, switchSubscribedNodeId = nid
16261627
#endif
16271628
}
16281629
liftIO $ writeIORef weakSelf =<< evaluate =<< mkWeakPtrWithDebug subscribed "switchSubscribedWeakSelf"
@@ -1667,6 +1668,9 @@ getCoincidenceSubscribed c sub = do
16671668
scheduleClear innerSubdRef
16681669
weakSelf <- liftIO $ newIORef $ error "getCoincidenceSubscribed: weakSelf not yet implemented"
16691670
(subs, slnForSub) <- liftIO $ WeakBag.singleton sub weakSelf cleanupCoincidenceSubscribed
1671+
#ifdef DEBUG_NODEIDS
1672+
nid <- liftIO newNodeId
1673+
#endif
16701674
let subscribed = CoincidenceSubscribed
16711675
{ coincidenceSubscribedCachedSubscribed = coincidenceSubscribed c
16721676
, coincidenceSubscribedOccurrence = occRef
@@ -1677,7 +1681,7 @@ getCoincidenceSubscribed c sub = do
16771681
, coincidenceSubscribedInnerParent = innerSubdRef
16781682
, coincidenceSubscribedWeakSelf = weakSelf
16791683
#ifdef DEBUG_NODEIDS
1680-
, coincidenceSubscribedNodeId = unsafeNodeId c
1684+
, coincidenceSubscribedNodeId = nid
16811685
#endif
16821686
}
16831687
liftIO $ writeIORef weakSelf =<< evaluate =<< mkWeakPtrWithDebug subscribed "CoincidenceSubscribed"
@@ -2051,12 +2055,13 @@ newtype EventSelector x k = EventSelector { select :: forall a. k a -> Event x a
20512055
newtype EventSelectorG x k v = EventSelectorG { selectG :: forall a. k a -> Event x (v a) }
20522056

20532057
fanG :: (HasSpiderTimeline x, GCompare k) => Event x (DMap k v) -> EventSelectorG x k v
2054-
fanG e =
2058+
fanG e = unsafePerformIO $ do
2059+
ref <- newIORef Nothing
20552060
let f = Fan
20562061
{ fanParent = e
2057-
, fanSubscribed = unsafeNewIORef e Nothing
2062+
, fanSubscribed = ref
20582063
}
2059-
in EventSelectorG $ \k -> eventFan k f
2064+
pure $ EventSelectorG $ \k -> eventFan k f
20602065

20612066
runHoldInits :: HasSpiderTimeline x => IORef [SomeHoldInit x] -> IORef [SomeDynInit x] -> IORef [SomeMergeInit x] -> EventM x ()
20622067
runHoldInits holdInitRef dynInitRef mergeInitRef = do

0 commit comments

Comments
 (0)