wavesoft / dot-dom

.dom is a tiny (512 byte) template engine that uses virtual DOM and some of react principles
Apache License 2.0
806 stars 53 forks source link

Performance issue #60

Open mindplay-dk opened 4 years ago

mindplay-dk commented 4 years ago

I noticed that the setState callback currently updates not only the affected node, but all of it's siblings.

https://github.com/wavesoft/dot-dom/blob/ddd6d50f9609b97f79ee9af10387598cfa19e7a5/src/dotdom.js#L134

State updates are synchronous, so imagine you have to update 10 sibling components - each component calls setState and triggers updates of all the other component, so a total of 10*10 = 100 updates.

Since the function signature is render(vnodes, dom), it looks like there's currently no way to update an individual child node.

Likely this needs to be refactored, something like extracting a function renderNode(vnode, domParent, domChild) and calling that from the setState closure instead?

wavesoft commented 4 years ago

This is unfortunately true...

I started doing some benchmarks with js-framework-benchmark and noticed a particularly bad behaviour on bulk updates. That's the same issue as you observed.

Indeed, some serious refactoring will be required, since this will definitely exceed the size budget 😞

mindplay-dk commented 4 years ago

...this will definitely exceed the size budget 😞

Okay, so, just a thought, but I was able to remove all of this:

https://github.com/wavesoft/dot-dom/blob/ddd6d50f9609b97f79ee9af10387598cfa19e7a5/src/dotdom.js#L76-L95

And this:

https://github.com/wavesoft/dot-dom/blob/ddd6d50f9609b97f79ee9af10387598cfa19e7a5/src/dotdom.js#L272-L284

All without affecting the core functionality - which makes me think, the core library itself has no dependency on this, and JSX works fine without out, so does this have to be part of the core library?

It seems you could push this to a plugin and give yourself a little more elbow room, so we don't trade-off performance for a few extra bytes saved?

wavesoft commented 4 years ago

Yes, you are right. This is just implementing the Declarative DOM Composition extension and is not part of the core library.

I haven't gone through the benchmarks yet, so I haven't really compared the final results of JSX versus declarative DOM. If JSX is a clean winner on the final size, I will consider removing it.

However so far this:

div.a("Hello")

Compresses more efficiently than this:

H("div", {className: "a"}, "Hello")

But on an actual project, the repetitive H("div",{className: " chunks could be nicely compressed, so the outcome is not clear.

wavesoft commented 4 years ago

But yea, if this allows us to achieve better reconciliation performance, I will consider it.

mindplay-dk commented 4 years ago

I'd expect the declarative DOM syntax would compress better, but someone (me) might still choose JSX perhaps mostly out of preference - so it would be nice if this API was optional either way.

I mostly suggested it as a work-around to your painstaking 512 byte limit 😁

mindplay-dk commented 4 years ago

This may be slightly off-topic, but here's my unminified version so far, if you're interested. I dropped the proxy stuff and turned it into an ES6 module. Currently clocks in at 519 bytes, I'm sure that's completely unacceptable to you. 😜

Jemt commented 3 years ago

It would be interesting to see how .dom stack up against other frameworks. I have previously used https://github.com/mathieuancelin/js-repaint-perfs (online demo here) myself to test a simple templating engine I built.