Caching of Surfaces does not persist [$5]
palf opened this issue · 14 comments
I created a simple animation example, then added a large background to the scene. FPS dropped by a factor of 4. Profiling shows around 60% of application time spent in the getSurface function, specifically in surface <- liftIO $ Cairo.imageSurfaceCreateFromPNG src.
total time = 16.62 secs (16616 ticks @ 1000 us, 1 processor)
total alloc = 116,918,680 bytes (excludes profiling overheads)
COST CENTRE MODULE %time %alloc
getSurface FRP.Helm 59.9 0.7
renderElement FRP.Helm 23.1 18.7
render.\.\.\.\ FRP.Helm 8.6 0.2
render' FRP.Helm 3.8 0.0
run''.\ FRP.Helm 1.1 0.0
withTransform FRP.Helm 0.9 1.2
getKeyState.\ FRP.Helm.Keyboard 0.4 57.0
toFps Main 0.2 5.9
run' FRP.Helm 0.1 7.9
times FRP.Helm.Animation 0.0 2.8
Investigation shows the first call to Map.lookup on each frame returns Nothing, whereas subsequent calls for the same Surface within that frame are cached correctly. On the next frame, the cache no longer holds the Surface, so we're creating Surfaces for each image file every frame.
Resolving this would provide a great performance boost for games with large assets.
--- Did you help close this issue? Go claim the **[$5 bounty](https://www.bountysource.com/issues/3596115-caching-of-surfaces-does-not-persist?utm_campaign=plugin&utm_content=tracker%2F290443&utm_medium=issues&utm_source=github)** on [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F290443&utm_medium=issues&utm_source=github).Good catch. I'll check it out.
Looks to me like the result of the StateT being run is just being thrown away in favour of the Cairo container monad and hence the engine state is not carried on to the next frame. Although I'm not all too familiar with the State monads, can someone back me up on this?
That's the same conclusion I came to; I wasn't able to resolve it, as I've no familiarity with State monads either.
That sounded more helpful in my head :)
The root of the problem is this line:
https://github.com/switchface/helm/blob/eb65f44a6432a9fc394e4b80c86c7a63d749e4c9/src/FRP/Helm.hs#L148
The state is lost after each render. render needs to have the StateT in its type. Correcting all the other usages so that it builds should then correct this issue (the rollover effect is that run, run' and run'' all need to be pulled into StateT and run should call evalStateT).
I can submit a patch if setting up the devenv is easy. Never tried.
If you're on Linux or OSX, it's easy. Windows is still a WIP/annoying because the SDL DLLs refuse to be found by the SDL2 bindings unless you edit the cabal file manually.
This actually would have been broken by the guy who did the StateT transition and got the $5 bounty.
hue #6 .
Oh well. Added a $5 bounty to this for parity. If you end up fixing it, it's yours.
This fix might work, but I think it's largely incorrect. Probably should have just bubbled the state higher and moved the evalStateT appropriately.
When done correctly, render would have the type:
render :: Element -> StateT Engine IO ()
Ugh, I tried to do that but I made a silly mistake now that I think about it that made me think it was hard to do. I only did it this ugly way because I had to release the patch ASAP. I'll update it with the next patch.
I've finished the patch, trying to build helm now ... found the instructions, installing everything takes a little while. Very dependency heavy.
@kasbah
Ensuring you've installed the profiling libraries, you can build a performance profile with the following commands:
build: cabal build --ghc-options="-prof -auto-all"
run: "path/to/executable" +RTS -p -s
This creates a file *.prof which will tell you where your application spends its time. I also installed Helm from a local build with profiling turned on, so my report contains information about Helm calls as well as local src calls.
You can get instructions here: http://book.realworldhaskell.org/read/profiling-and-optimization.html
Thanks!