Poor performance with SVG and listWithKey

[gergoerdi]

Doug asked me to post this even with the state the code is currently in (which is far from optimal), so here goes...

My code is in https://github.com/gergoerdi/tfb-reflex-repro and to reproduce it, just do a stack build in that directory, then navigate to the resulting index.html. (You'll need to edit the path in Main.hs because that's how hacked-together the source is at the moment).

If you then click on the Play button, everything works as expected, until the pace picks up and there are lots of rectangles flying around. That's when the framerate goes to crap really fast.

For comparison, I have the original Elm implementation running on http://unsafePerform.io/projects/tfb/ (but that one uses an HTML Canvas instead of SVG -- I'm not sure if that difference matters).

[oliver-batchelor]

You might try instead of using listWithKey is to flatten out the structure using 'dyn', listWithKey is rather of heavy if all the elements are changing all of the time.

-- Directly take timestamp instead toMark :: TimeStamp -> m ()

marks = void $ mapDyn (traverse toMark) ts >>= dyn

Just a hunch here - listWithKey can be expensive for a few reasons, still compiling GHCJS via stack so I couldn't tell you exactly if this is the case yet.

On Sun, Mar 13, 2016 at 2:50 AM, Dr. Gergő Érdi notifications@github.com wrote:

Doug asked me to post this even with the state the code is currently in (which is far from optimal), so here goes...

My code is in https://github.com/gergoerdi/tfb-reflex-repro and to reproduce it, just do a stack build in that directory, then navigate to the resulting index.html. (You'll need to edit the path in Main.hs because that's how hacked-together the source is at the moment).

If you then click on the Play button, everything works as expected, until the pace picks up and there are lots of rectangles flying around. That's when the framerate goes to crap really fast.

For comparison, I have the original Elm implementation running on http://unsafePerform.io/projects/tfb/ (but that one uses an HTML Canvas instead of SVG -- I'm not sure if that difference matters).

— Reply to this email directly or view it on GitHub https://github.com/reflex-frp/reflex/issues/46.

[gergoerdi]

At the Hackathon yesterday, Doug brought up

marks = void $ dyn =<< mapDyn (mapM toMark . filter (< horizon)) ts

as a possible alternative, but saying that listWithKey is supposed to be the better-performing version. Indeed, with that version (and of course changing toMark to TimeStamp -> m () as you mentioned), the frame drops are much worse.

With your version:

marks = void $ mapDyn (traverse toMark . filter (< horizon)) ts

I am not getting any marks.

[oliver-batchelor]

You forgot the >>= dyn at the end, so our (bad suggestions) should be exactly the same since mapM = traverse. ('mapDyn' is only useful if you use the Dynamic it produces, but 'dyn' is the reflex-dom function which does the switching)

It is interesting to hear that it is much worse, I guess the bottleneck is not what I expected. Given this is the case I wonder if the issue is actually just the fact it's just changing lots of elements causing lots of browser recomputation (Elm is benefiting from animaiton-frame rendering here).

On Sun, Mar 13, 2016 at 3:56 PM, Dr. Gergő Érdi notifications@github.com wrote:

At the Hackathon yesterday, Doug brought up

marks = void $ dyn =<< mapDyn (mapM toMark . filter (< horizon)) ts

as a possible alternative, but saying that listWithKey is supposed to be the better-performing version. Indeed, with that version (and of course changing toMark to TimeStamp -> m () as you mentioned), the frame drops are much worse.

With your version:

marks = void $ mapDyn (traverse toMark . filter (< horizon)) ts

I am not getting any marks.

— Reply to this email directly or view it on GitHub https://github.com/reflex-frp/reflex/issues/46#issuecomment-195860565.

[gergoerdi]

What do you mean by animation-frame rendering? What exactly is Elm doing fundamentally differently here?

[gergoerdi]

Oh and BTW I believe my taps only changes at 60 Hz (driven by the tick) so it shouldn't be doing unnecessary updates except once per (target) frame.

[oliver-batchelor]

There's a function called requestAnimationFrame where you give it a callback, and the browser calls this callback at some rate. You can update the state of the DOM during this callback without triggering browser re-computations. So you can use it to do all your updates in one transaction, and the browser won't re-draw in the middle of it. Where as if you just update the DOM many times, potentially every modification might cause some (expensive) re-computation.

https://developer.mozilla.org/en-US/docs/Web/API/window/requestAnimationFrame

Reflex-dom's updates to the DOM are all independent, so it's potentially an issue.

On Sun, Mar 13, 2016 at 11:37 PM, Dr. Gergő Érdi notifications@github.com wrote:

Oh and BTW I believe my taps only changes at 60 Hz (driven by the tick) so it shouldn't be doing unnecessary updates except once per (target) frame.

— Reply to this email directly or view it on GitHub https://github.com/reflex-frp/reflex/issues/46#issuecomment-195931097.

[gergoerdi]

Would it be possible / would it make sense to provide the ability to create an Event t () coming from requestAnimationFrame? If I did everything in response to that (at the current stage, that would just mean that timestamp would be tagged by that Event), would that help?

[oliver-batchelor]

It could work that way, it might be the simplest thing to work - what it would require would be to modify the top level in the attachWidget function, to create a new event (newEventWithTriggerRef) and fire it during the requestAnimationFrame, e.g. with fireEventRefAndRead.

The event could be then passed around as part of the GuiEnv record. Provided the event can be fired from the same thread it'll work out, which might be a bit messy getting it to work with both GHCJS and native WebkitGtk versions.

The way which we've discussed has been to move to a pull based renderer, where instead of immediately making changes using an Event, changes are accumulated locally as a Behavior which is sampled during a requestAnimationFrame (and in turn fire an event allowing the Behaviors to clear the render state)

I've a prototype hanging around which does just this - but I've had no time for it all lately.

On Mon, Mar 14, 2016 at 12:01 AM, Dr. Gergő Érdi notifications@github.com wrote:

Would it be possible / would it make sense to provide the ability to create an Event t () coming from requestAnimationFrame? If I did everything in response to that (at the current stage, that would just mean that timestamp would be tagged by that Event), would that help?

— Reply to this email directly or view it on GitHub https://github.com/reflex-frp/reflex/issues/46#issuecomment-195935571.

[gergoerdi]

OK so I'd love to try out a super-ghetto-lowtech version of that by just doing

refresh :: (MonadReflexCreateTrigger t m, MonadRef m, MonadIO m, Ref m ~ Ref IO) => Window -> m (Event t ())
refresh win = do
    (event, ref) <- newEventWithTriggerRef
    cb <- liftIO $ asyncCallback1 $ \timestamp -> do
        fireEventRef ref ()
    liftIO $ Window.requestAnimationFrame win $ Just $ RequestAnimationFrameCallback cb
    return event

and then tagging everything with that. However, that fails because the callback passed to asyncCallback1 needs to run in IO, not in m.

[ryantrinkle]

@gergoerdi You can use askPostGui and its result to invoke a GuiAction m from IO; see, e.g.: https://github.com/reflex-frp/reflex-dom/blob/develop/src/Reflex/Dom/Class.hs#L123

[gergoerdi]

But of course!

So now I have

refresh win = do
    (event, ref) <- newEventWithTriggerRef
    postGui <- askPostGui
    rec cb <- liftIO $ asyncCallback1 $ \timestamp -> do
            next
            putStrLn "about to fire the event"
            postGui $ void $ fireEventRef ref ()
        let next = Window.requestAnimationFrame win $ Just $ RequestAnimationFrameCallback cb
    liftIO next
    return event

This one does print "about to fire the event" repeatedly to the console, but it still doesn't seem to trigger the Event: the following code shows a steady 0.

main :: IO ()
main = mainWidget $ do
    Just win <- liftIO currentWindow
    tick <- refresh win
    display =<< count tick

[dimsuz]

Hi! @gergoerdi did you solve this somehow? I am about to try reflex for some svg animation rendering and I'd like to make sure it uses requestAnimationFrame too.

[gergoerdi]

@dimsuz: I got requestAnimationFrame working with help from this SO answer. I haven't had time to work on my original problem since then, but just trying out the technique from that SO answer has already improved the fluidity of my animation.

Once I'll be able to put a bit more work into it, I'll get back to you with further results.

[dimsuz]

Thank you! It would also be nice if reflex incorporated this somehow, this is often used for animations and other timing sensitive stuff in modern web apps.

On Mon, May 2, 2016, 13:21 Dr. Gergő Érdi notifications@github.com wrote:

@dimsuz https://github.com/dimsuz: I got requestAnimationFrame working with help from this SO answer http://stackoverflow.com/a/36704222/477476. I haven't had time to work on my original problem since then, but just trying out the technique from that SO answer has already improved the fluidity of my animation.

— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/reflex-frp/reflex/issues/46#issuecomment-216208613

[ryantrinkle]

This issue was moved to reflex-frp/reflex-dom#79