Aggressive inlining can make elerea misbehave #5

Merged
merged 1 commit into from Apr 1, 2012

Projects

None yet

2 participants

@takano-akio
Contributor

The following program works correctly when compiled with GHC 7.0.4 and '-O2'. However, when it is compiled with '-O2 -funfolding-use-threshold=100', it prints "0 1 2 2 2" instead of "0 1 2 3 4".

import FRP.Elerea.Simple
import System.Mem

main :: IO ()
main = do
  sample <- start $ listToSignal [0..]
  sample >>= print
  sample >>= print
  performGC
  sample >>= print
  sample >>= print
  sample >>= print

listToSignal :: [a] -> SignalGen (Signal a)
listToSignal xs = fmap (fmap head) $ stateful xs tail

I believe this problem is caused by a bad interaction between inlining and weak pointers. In Simple.hs, addSignal calls mkWeak to create a weak pointer whose key is the new signal, but if sig gets inlined into the call to mkWeak, it ends up creating a weak pointer whose key is a lambda. Since the lambda is referenced from nowhere else, it will soon get garbage-collected, causing the weak pointer to be invalidated prematurely.

My commit tries to fix the problem by marking sig NOINLINE. It makes the above example work, but I'm not 100% sure that this is the correct fix.

@takano-akio takano-akio Added NOINLINE pragmas to ensure mkWeak creates correct weak pointers
If 'sig' gets inlined into the argument position of mkWeak, mkWeak
will create a weak pointer whose key is a lambda. Since there is
no other reference to the lambda it will soon get garbage-collected,
causing the weak pointer to be invalidated prematurely.
04601e5
@cobbpg
Owner
cobbpg commented Mar 27, 2012

Yeah, I had all sorts of problems similar to this in the early phase of development. For instance, even though Signal is a newtype, it still seems to matter whether the S constructor is included in the target of the weak pointers. And the library breaks with 7.4.1 due to another inlining related issue.

@cobbpg cobbpg merged commit 3c28daf into cobbpg:master Apr 1, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment