urweb / urweb

The Ur/Web programming language
http://www.impredicative.com/ur/
Other
807 stars 66 forks source link

Using virtual dom to optimize clientside rendering? #113

Open FrigoEU opened 6 years ago

FrigoEU commented 6 years ago

Hi,

First of all I'd like to say I think Ur/Web is an incredible project. So much thought has been put into this! Unfortunately I've only now found this project. It's very silent and under the radar, which is a pity because it offers so much!

I've been reading the documentation and going through the tutorial at /ur/demo, and I came across this in the manual (page 47), in regards to the \<dyn> tag:

The semantics of \<dyn> tags is somewhat subtle. When the signal associated with such a tag changes value, the associated subtree of the HTML page is recreated. Some properties of the subtree, such as attributes and client-side widget values, are specified explicitly in the signal value, so these may be counted on to remain the same after recreation. Other properties, like focus and cursor position within textboxes, are not specified by signal values, and these properties will be reset upon subtree regeneration. Furthermore, user interaction with widgets may not work properly during regeneration. For instance, clicking a button while it is being regenerated may not trigger its onclick event code.

I believe I've also stumpled upon a problem with the same root cause in the TodoMVC demo, where I've found it impossible to click on the delete button because the button keeps getting removed and added.

The above mentioned problem is exactly the reason why JavaScript frameworks like React use something called a "virtual dom". Probably you're familiar with the concept, but in short it optimizes DOM updates by comparing the previous and current versions of a data structure (= the virtual dom) which represent the actual DOM. These libraries take great care not to remove and then re-add dom nodes that stay the same when rerendering the UI. I believe this could alleviate much of the problems specified above.

There are a wide range of libraries implementing such an algorithm, but since Ur needs only the most basic version of this, I'd suggest something like https://github.com/yelouafi/petit-dom, which is extremely fast and implements not much more than just the diffing and patching algorithms.

I have of course no idea of the internals of Ur/Web or of the way it handles \<dyn> tags at the moment and whether it would be compatible with virtual dom libraries, but I thought it would be valuable to at least propose it.

FrigoEU commented 6 years ago

Another library that could be evaluated is Google's incremental-dom (https://github.com/google/incremental-dom) which is not designed to be handwritten and focuses on performance and memory usage in particular.

FrigoEU commented 6 years ago

I checked the TodoMVC example specifically, and that problem has to do with the way it's implemented with onmouseenter and onmouseleave handlers. I don't think it'll ever work because whenever you add the delete button and your mouse is over it, the onmouseleave handler will trigger and remove the button again. I checked some other implementations and they all use simple CSS to show and hide the delete button on hover. I would send a PR for it but I don't see it on Github.

I'm also working on a PR to show what I mean about virtual dom, using https://github.com/patrick-steele-idem/morphdom. This library works well because it takes the dom elements to be created as strings, so code changes are minimal. I got it working for the general case in "x.recreate()", but having some trouble with the special cases for table and tr.

ashalkhakov commented 6 years ago

This would be tremendous!

ashalkhakov commented 6 years ago

Another idea is improving the client-side interpreter. I don't understand how it works, and Adam did not comment on it when asked.

FrigoEU commented 6 years ago

Is there anything specific that you know needs improvement?

ashalkhakov commented 6 years ago

I don't have a benchmark, just wrote a few simple apps to test things out:

https://github.com/ashalkhakov/urweb-projects/blob/f3f2ccc8806c1d5d5c88c32eeffd0f3e39f338da/sam/sam.ur

and it seems to me that the current design, where the whole view (HTML) is re-rendered from scratch every time is faster than nested dyn tags. At least it seems more responsive to me.

Ur/Web's client-side interpreter adds some overhead. First point: see this code, where every function is curried (so, every Ur/Web function when translated to JS is curried, except some primitive functions/builtins). Second point: preemptive threads in Ur/Web are implemented non-natively.

Also, it looks like when the Ur/Web code modifies a "source", all signals attached to that source are immediately recomputed. This can be unacceptable in some scenarios (say you have some heavy computation in the signal attached to a source). It would be better to adopt SAM-style reactive loop, where the (possibly mutable) model is updated with new values, and then the UI is updated (SAM advocates distinct propose-present-learn phases). OTOH, Ur/Web seems to optimize pure functions very aggressively...

achlipala commented 6 years ago

Thanks for all the thoughts on improving Ur/Web! I'm hesitant to add a dependency to some other JavaScript library, especially before seeing a clear case where a real application is running into performance problems. The issue with node state not captured by the Ur/Web API has bothered me for a while, but I might be tempted to instead allow connecting sources to those state components.

FrigoEU commented 6 years ago

Fair point, clientside perf of urweb seems good, so even though this makes it even better, it's not needed.

My reason of implementing this was more that you could put some more stuff in dyn tags without being afraid that dom elements would lose state, focus, etc.

The dependency is very small, and basically it's a better .innerHTML so not too complicated. The urweb instance I'm using now to build my project is my fork and it works well and I'll definitely keep using it. But point taken, totally fair that you don't want to take on the dependency (at the moment?).

On Jan 31, 2018 7:37 PM, "Adam Chlipala" notifications@github.com wrote:

Thanks for all the thoughts on improving Ur/Web! I'm hesitant to add a dependency to some other JavaScript library, especially before seeing a clear case where a real application is running into performance problems. The issue with node state not captured by the Ur/Web API has bothered me for a while, but I might be tempted to instead allow connecting sources to those state components.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/urweb/urweb/issues/113#issuecomment-362028130, or mute the thread https://github.com/notifications/unsubscribe-auth/AEQbgn4Qhjliz76LIPF9cj8xjRxLUyB6ks5tQLLigaJpZM4Rp409 .

vizziv commented 6 years ago

Is this something that could potentially be enabled/disabled in a project's .urp file? There's some issues with nested .urp files, I suppose, but not having to worry about focus is appealing.

FrigoEU commented 6 years ago

@vizziv One way how I can see that being possible would be if you could specify the location for the urweb.js runtime library in an .urp file. Not a big fan of that personally since the C and JS runtimes are quite strongly coupled together.

achlipala commented 6 years ago

I'm coming back to piled-up Ur/Web issues after a busy semester worth of distractions. Two points:

  1. The TodoMVC demos do work for me. I only test in Firefox most of the time, and that led me to miss an incompatibility with Chrome. Interestin'. My somewhat unproductive thought about this example is that I felt obligated to adopt the exact visual styling from other implementations, and my own preferred Ur/Web UI style wouldn't have run into the issue, being less "busy-body-oriented" in surveilling mouse movements to change styling.

  2. Yes, I continue to feel that it wouldn't be good to add a dependency on morphdom for Ur/Web "out-of-the-box." Just in case it wasn't already clear: by using many <dyn> tags, and by nesting them, it is possible to express fine-grained dependency structure, such that only those DOM nodes whose dependencies have changed will be regenerated. Others will definitely mantain all their state.

FrigoEU commented 6 years ago

After using my fork the last few months, I've become less enthusiastic about this change.

What I do like about it is that it allows you to be a bit more "sloppy" with you dyn tags, rendering some more stuff at once and having morphdom figure out the most performant way to update. Especially when you have big lists where only one thing changes this pays off quite nicely.

Why I've become less enthusiastic about it: It doesn't actually fix the problem with cinput tags losing state when you're rerendering them completely: The mainline version of Urweb first creates these tags as \ tags, then puts them into the dom and then "instantiates" them for lack of a better word. The morphdom version does the same thing, so it diffs the old but instantiated version against the new but \ version tag, thus losing some of the benefits. Instantiating the tag before diffing is I believe not possible since that might change existing behavior, but I haven't looked into it that far.

Personally I'm a bit on the fence. Having it as an optimization that can be triggered would be ideal in my opinion. I can definitely see a certain kind of apps that benefit from this: Showing a big list of things where only one changes at a time is ideal for morphdom to handle. The complexity it brings is not free though, and neither is the javascript size. For my current project I don't have the issues it solves: Big lists I try to render on the server and keep static, my more dynamic pages I take care to not show too many things, in most cases this lines up nicely with good UI design.

Anyway, just thought I'd drop my 2 cents after using it for a while.