crysalead-js / dom-layer

Virtual DOM implementation.
MIT License
30 stars 1 forks source link

better performance? #60

Closed ghost closed 9 years ago

ghost commented 9 years ago

Hi. Long time. I was study some code now, and compared yours with cito. I see that in render() you appendChild on each child. What if you have many thousands of children? Touching DOM a lot, or BANG! CitoJS I understand it, don't do this. It only update the children object, and attach the children with a insertChild() function if there is an parent node.

Couldn't this give a better performance for you???

I investigated further. In your patch() you checking for parentNode... CitoJS do it different, and only attach that child to the node if there is a parent. You insert on the node all the time and don't update the object.

Can this be done differently ??? Maybe you should investigate it??

REACT got a function too for merging two objects - used for testing so you can test the virtual node separatly before patching / attaching. Something?

It also seems to me that citojs updating it's object, not the DOM node and then insertion.

Btw. when will we see your Component? End of this month?

jails commented 9 years ago

I guess you are talking about appendChild in create().

Long story short, dom-layer allows to mount an array of nodes inside a single existing mount point like in https://jsfiddle.net/ammvj8u1/ . At some point all root nodes needs to be monted, there's not workaround. CitoJS doesn't support this feature and always assume you have one root node. So If you can handle one root node, dom-layer will work the same way as CitoJS.

For the patch.node() is was some unused code so I removed it updated the specs with the correct syntax in https://github.com/crysalead-js/dom-layer/commit/1b97d6c2510c344bdfdb72159ffc09ed533b903e

Not sure to get what you meant with REACT but all dom-layer specs are pretty much isolated and works with virtual node directly. Pretty much only the ones related to mounting require a DOM.

CitoJS is an interesting implementation but it can't be taken as an argument. Otherwise I wouldn't waste my time on dom-layer and would use CitoJS right away ;-). So please argue on the algorithm instead so I can understand what part you are talking about and why you think it should be the way to go.

Otherwise yeah the component layer should go out at the end of the month, and probably at the very end ;-)

ghost commented 9 years ago

I got your point and understand too, but still... I'm stupid when it comes to this, but at least I will try to explain. Your mounting point.... Let say the node you are mounting have 4 children. Why not deal with this 4 children first in memory , sorrt and arrange, and update? Then when that is done, you attach to parent node all of them at once. I assume you can use range() for this as REACT do too? For if you move the children in memory - same as REACT and CitoJS, you ar enot doing heavy shit on the DOM. That happen when you attach to parent node - the node you are mounting. Then again I guess you need to filter on this because nodeType 1 - not nodeType 3 - has children. But still why faster isn't??

So when you patch... Node 1 - 4 children like ( d, a, c , b) Node 2 - 2 children ( a, b ) Take out the children, to things with it, put it back and attach on the parent node? You got me? I'm probably stupid her. Sorry for that ! I will BANG my head sa wall! REACT got a method where your created object {
tagName: "div", children: [] } are accessible for end-user and you can do test on it to check your node befor ecreating anything. Then also got an merge function for two objects. I think at some point they use that merge function to merge the two nodes and take away the things not similar. And isolate the children too and run that separatly. Then also look into Kivi and the updateChildren method. Things you can use there.

jails commented 9 years ago

Let say the node you are mounting have 4 children. Why not deal with this 4 children first in memory

It's done in memory. When this https://github.com/crysalead-js/dom-layer/blob/master/src/tree/create.js#L12 is called, the DOM tree is built in memory through nodes[i].render(parent). And as long as your element doesn't have any ownerDocument attached it's fast and the DOM tree still a kind of "virtual DOM tree". And finally the "virtual DOM tree" is attached to the document when the container is the DOM mount point, so at the very last step. All virtual dom implementation works the same way, so there's nothing special in dom-layer on this point.

I'm not sure to understand your point on "if you move the children in memory - same as REACT and CitoJS". They are moving children in the DOM otherwise anything won't ever be updated in the document.

The patching algorithm is the following https://github.com/crysalead-js/dom-layer/blob/master/src/tree/patch.js#L16-81 and it's one of the fasted out there. But if you think you can make it faster please go ahead, and send a pull request ;-).

ghost commented 9 years ago

I'm thinking for the mounting, why not turn it up-side-down? In the mount() function recieve all nodes as an object as cito, still as an array. [ obj1, obj2, obj3]. Then mount and render and attach.

Similar to Cito, but have a look here too: https://github.com/greenish/react-mount/blob/master/react-mount.js#L173-L198

This is a REACT mount simplified. Wouldn't you get the single existing mount point?

I saw that code too, but I'm not a javascripter. I only know you can get it 20% faster. Why? https://github.com/localvoid/kivi/blob/master/src/vdom.js#L710-L761

I guess your code come from here? Same as cito, and all others?

jails commented 9 years ago

Turning what and why and for what point ? CitoJS starts with a single node then process the node's children (which is an array). dom-layer can starts with a single node, and array of nodes or a factory (i.e a function). But since the algorithm is recursive they all ends up to support array of nodes at some point.

In both cases arrays of nodes are processed the same. And the only difference with dom-layer is that arrays can occurs at a root level.

If you have something in mind just go ahead, it will probably help you up to understand why there's no real difference in regards to the patching step and why the dom-layer mounting strategy is more flexible than React or CitoJS.

ghost commented 9 years ago

I understand your mounting thing, but still not a javascripter :)

Still. I found the solution I hope, and I didn't get any BANG!! And this is how CitoJS do it!!

Okey, I understand your create() function. Cool!! And now try to follow me!

when you render the node there now, add a second argument, "container".

You know container will only appear once, right? So then you can create your node like citoJS does, and if container are present, appendChildren to that parentElement. Else only create the nodes and sort them before attaching them.

In this case, you will avoid re-use create inside the createNode function, and a lot of appendChildren().

And ofc. your patch method need to be changed. Say you have a child with sub-children?? The first child will then be the parent again, So then you only attach the sub-children on the parent child. But you are not touching DOM are you?

jails commented 9 years ago

Sorry but I think you are completly off base.

if container are present, appendChildren to that parentElement. Else only create the nodes and sort them before attaching them.

You don't get it, appendChild() is the only way to go. And CitoJS is doing it the same way. Once the root node is created all its children need to be appended to it otherwise the tree won't be rendered correctly. CitoJS does it here https://github.com/joelrich/citojs/blob/master/src/vdom.js#L796 . At this point domNode will be the parentNode, and appendChild() will be used to attach the subtrees. And if you add some console.log() around all appendChild() in CitoJS you will understand that it's not used only once at a "top level".

It's fast only because we are just playing with DOM nodes which are not inserted in the document (i.e not visible, so there's no need for repaint etc. so it's fast). It's not just creating DOM element, they need to be assembled with appendChild() before going live in a document.

So please open a PR because I don't understand your reasoning.