Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dom nodes aren't safe for memoisation #161

Closed
logaan opened this issue Jul 8, 2017 · 3 comments
Closed

Dom nodes aren't safe for memoisation #161

logaan opened this issue Jul 8, 2017 · 3 comments
Labels

Comments

@logaan
Copy link

logaan commented Jul 8, 2017

I'm not sure if it's a project goal to be safe for memoisation and please do let me know if it isn't. However I was surprised to see that the vdom nodes could not be re-used.

On this ClojureScript + DomVM implementation of TodoMVC pure functions are used to create domvm nodes. I re-render the whole view each time any part of the application's data changes. To avoid re-running view functions with data they've seen before I store the results and on repeated calls return the old answer.

You can see this in action by clicking on the Active link at the bottom and then back to the All page. Uncaught TypeError: Cannot read property 'previousSibling' of null is thrown.

@leeoniya
Copy link
Member

leeoniya commented Jul 8, 2017

You're correct. The only way to literally re-use the old vtree rather than regenerating and diffing against the old one is to return vm.node from a view's render(). This serves as an indicator of 0 changes and will short circuit any additional dom reconciliation (and hooks).

whether it's possible to add this guarantee to domvm is an interesting question, but it's not a trivial task since a vast part of the codebase assumes that vnodes are regenerated and immutable after creation; there are hooks that can set temporary transitional state on old vnodes for CSS animations, etc.

i'm inclined to say that this is not a worthwhile endeavor. the available optimizations in domvm at the view & node level are quite sufficient - it doesn't get much faster than domvm [1].

to avoid re-running view functions with data they've seen before I store the results and on repeated calls return the old answer.

this is extremely cheap, and in fact memoization can lead to GC sadness since all vnodes are linked to their parents and children. once you start retaining them, they start leaking rather than being efficiently discarded by the JS engine. a completely unoptimized dbmonster bench regenerates, diffs and reconciles ~2900 dom nodes in < 1.5ms [6]. With some basic optimizations it can get < 0.5ms [7].

@lawrence-dol used to memoise & reuse domvm vtrees heavily in his app prior to v2 and discovered that much of the rendering slowness was actually a result of this practice. [2].

if you're interested in various optimization facilities, you can ask about them here until their docs are ready (see #156). domvm v3 has a feature called lazyList which does reuse old vnodes from a uniform list, but it's a declarative type setup that would not work for doing your own old vnode returns. you can see how it's used in the bench [3] to reduce RAM usage, but it did nothing to boost the bench speed [4].

i should actually port @yosbelms's domvm v1 TodoMVC demo [5] to domvm v3.

[1] https://rawgit.com/krausest/js-framework-benchmark/master/webdriver-ts-results/table.html
[2] #116 (comment)
[3] https://github.com/krausest/js-framework-benchmark/blob/master/domvm-v3.0.1-keyed/src/main.es6.js#L92
[4] krausest/js-framework-benchmark#203
[5] https://github.com/leeoniya/domvm/tree/1.x-dev/demos/TodoMVC
[6] https://cdn.rawgit.com/leeoniya/domvm/3.x-dev/demos/bench/dbmonster/index.html
[7] https://github.com/leeoniya/domvm/blob/3.x-dev/demos/bench/dbmonster/app.js

@logaan
Copy link
Author

logaan commented Jul 8, 2017

Thanks for the thorough reply. I think I'll accept that the memoisation was premature optimisation and stick with the (already blazing fast) straight forward approach.

@logaan logaan closed this as completed Jul 8, 2017
@leeoniya
Copy link
Member

leeoniya commented Jul 8, 2017

FWIW, you would get a small speed boost by moving to v3 ;)

i just published v3.0.2. the needed changes from v2 - if any - are mostly mechanical (arg order, naming). the release log [1] might be helpful.

[1] https://github.com/leeoniya/domvm/releases

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants