Loading 1100 clickEl's is very slow

[mpdairy]

I have a list of 1100 users and I tried to make a button for each one (using clickEl) that returned the userId to a parent element, which I would use to jump to a new route. But it's extremely slow loading that many click elements. It could handle a couple hundred well, but approaching 1000 the performance really plummets. Once they load up, it's pretty responsive, but loading them take at least 10 seconds.

For now I just made a fake button function that uses a href instead of Concur's events machinery:

linkButton :: String -> String -> Widget HTML ()
linkButton label url =  el_ E.a [ A.href . JS.pack $ url ]
                        $ el E.button [ A.class_ "pure-button" ] [ text label ]

which is really fast and fine for now since I'm just jumping to an URL, but at some point I'm sure it would be nice/essential to be able to use clickEl's or some other event listener for a big list.

[ajnsit]

Thanks for reporting this. 10 seconds is just unacceptable! Is it possible for you to share the code?

[mpdairy]

Thanks, I'll make a simplified version that demos it and paste it here later today.

[mpdairy]

Ok here's a widget that will just load a bunch of buttons:

import Concur.VDOM ( HTML, button, el, el_ )

slowList :: [Int] -> Widget HTML Int
slowList = el E.div [] . fmap buttonize
  where
    buttonize n = el_ E.div [] $ do
      button $ show n
      return n

If I call it with slowList [0..500] it takes a couple seconds to load, but if I do slowList [0..2000] Chrome will eventually crash like the page is unresponsive, but if you click "Wait" it will eventually load.

[saurabhnanda]

Randomly listening to the conversation. What all is concur doing behind the scenes here?

On 21-Oct-2017 5:30 AM, "Matt Parker" notifications@github.com wrote:

Ok here's a widget that will just load a bunch of buttons:

import Concur.VDOM ( HTML, button, el, el_ )

slowList :: [Int] -> Widget HTML Int slowList = el E.div [] . fmap buttonize where buttonize n = el_ E.div [] $ do button $ show n return n

If I call it with slowList [0..500] it takes a couple seconds to load, but if I do slowList [0..2000] Chrome will eventually crash like the page is unresponsive, but if you click "Wait" it will eventually load.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ajnsit/concur/issues/6#issuecomment-338347901, or mute the thread https://github.com/notifications/unsubscribe-auth/AABu0SBXhu5jE3rQ9THllh68ET0STCxZks5suTQGgaJpZM4P__F6 .

[ajnsit]

@mpdairy I added the benchmark code here - https://github.com/ajnsit/concur-benchmarks/blob/master/src/ClickList.hs

To get another data point - Which browser are you on? From my quick testing, there seems to be a massive performance difference among browsers - Firefox (I use nightly) was able to load [0..5000] widgets in 1-2 seconds with performance becoming inconsistent beyond that. Chrome and Safari were both extremely slow in comparison (though neither froze).

@saurabhnanda There are a whole bunch of things happening because of the highly abstracted interface for Concur. Here's what I see as areas of potential improvement -

1. String packing/unpacking (JSS.String <-> String)

2. A button can have arbitrary children which may have their own events. So each button is effectively a <button> wrapper over a list of child widgets. We capture the events generated by the top level button and wrap them in a Left, and all the events from the child widgets in a Right. In this case, we only have static text as the contents of the button, but it's still treated as a full widget. Then the Left and Right events are joined with <|>. All this has some (non-trivial) overhead. I went ahead and removed this abstraction from text only buttons - https://github.com/ajnsit/concur/commit/4dbf7dc735ad79018ac97b3e0dce8b7bbec8e764.

3. The largest benefit would definitely come from optimising the render cycle. All the button widgets are orrd together sequentially, and their DOM views populated . This currently has no batching, and rapidly and repeatedly modifying the DOM has known performance implications.

4. There are a lot of STM threads created which may or may not have a significant performance impact.

5. There does not seem to be excessive thunking or memory leaks. But it's still an area to look into.

[ajnsit]

Okay I'm pretty certain I understand why this is happening. Will push a fix soon-ish.

[ajnsit]

@mpdairy I just pushed a change that fixes this in concur-core and then specifically for concur-vdom text button widgets. Please try it and let me know if that fixes the issue for you. I'll soon push fixes for other inbuilt widgets (it's a trivial, but tedious change).

[ajnsit]

Keeping this open until all the widgets are converted.

[ajnsit]

I uploaded the benchmark demos here - https://ajnsit.github.io/concur-benchmarks/

[ajnsit]

@saurabhnanda BTW I added another benchmark where I pretty much copied your JS benchmark code for miso, and tweaked it a bit to get it to compile with concur. You can check the source and demo.

[mpdairy]

Hey thanks! That update made my button list really fast. Even the list with 4500 users loaded quickly. I didn't measure it, but the new buttons seem just as fast as the fake linkButton button I pasted at the top of this thread. What was the change you made to speed them up so much?

[ajnsit]

Hah well, it's really a new feature, and I'm glad it solved your problem! The changes are all here - https://github.com/ajnsit/concur/commit/78ac2b224d1d2454ad4ddb014bc48950cc46481a.

Gist

Basically I added support for blocking IO to Concur. Then used it for a very common use case where async io was a bottleneck.

Background

All actions like liftIO and liftSTM are run async, and are effectively full widgets.

A common pattern with a lot of widgets (like button) was to use liftSTM to procure a Notify (a very quick operation), and then used the Notify to link the view click and the handler. This means that a single Button was effectively a sequence of two widgets, liftSTM and then the actual button UI.

Also all widgets are encapsulated and run async, and any changes to the widget tree causes a full dom render cycle (albeit with virtual-dom diffing).

Problem

So when a button is first added to the widget tree, it actually renders the liftSTM widget (i.e. no UI), which quickly finishes, and then renders the actual button UI.

When you add two buttons in quick succession, the first liftSTM widget renders and finishes, then the second liftSTM widget renders and finishes, and then the first button UI renders, and then the second button UI renders. The order can vary depending on how long it takes for a liftSTM to finish.

You see where this is going.

With 1100 button widgets, you have 1100 quick dom rerender cycles in a row which takes ~10 seconds on your machine, including the overhead for all the rapid widget sequencing.

Solution

I added a special function called awaitViewAction which provides a Notify to a widget, but does so with blocking IO. Blocking IO prevents DOM updates so cannot be used for long running IO actions, and hence I am reluctant to make it available to clients. However this specific use case of constructing a Notify in STM is guaranteed to be extremely short and can safely be exposed to client widgets.

With this change, on adding 1100 button widgets, the IO actions are quickly run in succession without touching the DOM, and finally each widget constructs its own UI which is populated into the DOM at once (one render total for all 1100 buttons). The same as what happens with your "fake" linkButton code.

[buggymcbugfix]

Just reading this out of curiosity—I'm looking for a framework to do some work in and Concur is looking amazingly straightforward to use—it's either too good to be true or @ajnsit is a world-class API designer! ;)

At any rate, you can probably close the issue?

[ajnsit]

Appreciate the positive words! The API really is that good. I'm yet to find a usecase where it doesn't do a better job than any other general purpose UI lib. I wish more people would use it and provide feedback so I can make it better. Please use it, and I would be happy to help out with any issues you might encounter.

[ajnsit]

Oh and I don't want to close the issue until I have fixed similar performance issues with all the widgets in the library. Unfortunately I got a bit sidetracked with the purescript bindings for Concur, but I plan to come back and work on this lib more.

[buggymcbugfix]

I'm definitely keen on using it. I need to write a small app for someone and am planning to use concur if I can make it work.

[ajnsit]

Sounds good 👍

[mpdairy]

Concur is great and makes it easy to do what would otherwise be really complicated. Though one pain point for me, last time I used it, was in making text input boxes that updated some state at every keystroke. I had to make some hacky thing that used an IOref to hold the form state and then make special text input widgets that mutated the state as the user typed, because if a widget returns anything it gets re-rendered and the input focus is lost. It would be nice if focus somehow remained after a widget returned a value, if it was still there, though I don't know how it could done, or determined that it was the same widget.

[ajnsit]

Oh yeah, that was a problem with ghcjs-vdom, but react solves it pretty nicely now. In purescript-concur (which also uses a react backend) the text widgets return values without losing the input focus. For example look at the color demo here - https://ajnsit.github.io/purescript-concur/. And the source - https://github.com/ajnsit/purescript-concur/blob/master/examples/Test/Color.purs.

I'm surprised I haven't already created nice text widgets for concur-react. I will add that to my todo list!

[mpdairy]

Oh wow, that color demo is great! Adding that to concur-react would be really helpful.