[WRONG] poor performance

[ghost]

Hi. Notice that this lib all over have a poor performance. Use of a constructor are terrible slow. Without RAF or setTimeOut the render take 15ms. Are this going to be fixed?

Too much overhead.

Use of 'this' keyword everywhere are also a huge performance killer. And so are use of 'new' keyword too.

[gaydenko]

...huge performance killer. And so are use of 'new' keyword too.

I guess the thesis is a little overstatement :) Say, another virtual DOM implementation (Trackira) creates all nodes with new and still one of the fastest.

[ghost]

@gaydenko well, I checked just now. And for creating nodes yes you are right. But this guy creating a prototype for whatever he does!!! Even when setting attributes, there is an constructor. In the patch algorithm, for each move, insert etc. operation there is an constructor. This is the first library I have seen doing this!
And even this Trackira don't do that.

Summary. For whatever are done, he creates a prototype. Even the component, to get that to work you need to use the new keyword in front of it. Why? It's a prototype too!!

[dfilatov]

Hi, @marcorra.

Without RAF or setTimeOut the render take 15ms

What does it mean? How do you measure it? Could you give me your test?

Use of 'this' keyword everywhere are also a huge performance killer. And so are use of 'new' keyword too.

Сompared to what? Could you provide any proves/tests?

Even the component, to get that to work you need to use the new keyword in front of it

As user you don't need to use new keyword to create component. A component is instantiated inside the engine. To use component you should create a corresponding node, for instance vidom.createNode(Component, ...).

Ps. There is comparison with Trackira on my notebook:

[ghost]

First I had to check if your benchmark was correct. It's not. In your main.js you need to create a span node with a text node as a child. You have only a text node. Check all other equal setups, and you will see that they are doing this. You are not.

So you will fail almost every test. Obiously without knowing it, for the benchmark will work for you, but still fail silently. We call that cheating !

Anyway. Just add

console.log(this.container.innerHTML); debugger;

after each in the render. Nothing are inserted to DOM.

Easy fix for this. Remove RAF and setTimeout, and you will get the real performance. Wich will show you that the render are sky high over what you show here.

The benchmark are not build to work with RAF or setTimeout. Just ask the author of it if you doubt that. You should also get this message in your console.log:

Each child in an array should have a unique "key" prop. Check the render method of ResultsTable. See http://fb.me/react-warning-keys for more information.

Because nothing are inserted into DOM.

About my other things.

A killer? Well, I don't need to give a test. Google should be enough. But have a look at this jsperf: http://jsperf.com/object-notation-vs-constructor/31

You see who is slowest? So doing this in every iteration or what so ever, it will slow things down dramaticly.

Usefull articles.
http://www.toptal.com/javascript/javascript-prototypes-scopes-and-performance-what-you-need-to-know

[dfilatov]

I've added console.log(this.container.innerHTML); debugger; and I see inner HTML.
Are you sure you use https://github.com/dfilatov/vdom-benchmark-vidom for your tests?
If you look at code https://github.com/dfilatov/vdom-benchmark-vidom/blob/master/web/js/main.js#L40 you see there's used sync version, without RAF, timeout and whatever.

About message:

Each child in an array should have a unique "key" prop. Check the render method of ResultsTable. See http://fb.me/react-warning-keys for more information.

My library doesn't have such a message.
It seems you test another library in your test.

[ghost]

I downloaded everything to my computer and run it from here. What browser are you using? And what do you see in your log?

Try to add this

children.push(vidom.createNode('span').key(n.key).children(vidom.createNode().text(n.key.toString())));

look into your console again.

I get (an empty string) in FF console.log when I do that console.log checking. Meaning nothing are inserted.

You should also use this page to verify that your tests are working

http://localhost:3000/?data=http://vdom-benchmark.github.io/vdom-benchmark/generator.js&test=true

[dfilatov]

I use FF and see html in my console.
Could you explain how you install https://github.com/dfilatov/vdom-benchmark-vidom?

[ghost]

I download it. First I download the vdom-benchmark, and edit the config so you are coming on the list there. Then I run the gulp command. I'm using a locale url to point to your benchmark on my harddrive - localhost - . Then I just compile your benchmark and go to the build folder and run the tests.

[dfilatov]

[ghost]

Me too got that now, but with various results here. I will investigate further. And you have done a great work!! No doubt! But before you PR for adding it to the benchmark, need to be sure it's correct. Add at least a span tag as a parent with a text node as it child node. And I will check out more of this later on to give you more info.

With a more correct result now, I see there are parts that you need to improve to get even better performance. And then again you need to re-consider the constructor use.

And I see you are using the old benchmark. But your overall time in my FF now are 45449. Trackira's overall time are 37009

[dfilatov]

I've added <span/> node and commited to repo.
There's new results:

[dfilatov]

With a more correct result now, I see there are parts that you need to improve to get even better performance.

Could you suggest something except not using constructors?

By the way http://jsperf.com/object-notation-vs-constructor/31 doesn't work at all.

[ghost]

Now I tend to agree that the benchmark are more correct :)
jsperf are up and down since it come back last night. It will show you that constructors are slowest, then array and fastest object.

Suggestions? I don't know :) You should re-consider all your use of constructors like in setting attributes etc. Even REACT now change reactElement to a pure object. At least there are a non-closed PR for that. And moving away from constructors.

Maybe a sort of class system with inheritance? So you don't need to use the new keyword or instanceOf?

[dfilatov]

Maybe a sort of class system with inheritance? So you don't need to use the new keyword or instanceOf?

I can't understand how to use class system with inheritance without using "new" keyword.

[dfilatov]

I've closed this issue as not correct. You're always able to open new issues with particular problems.

[ghost]

I was reading this chaos... What about a internal register with a unique identifier on each node in replacement for a constructor? Example on each iteration in the patch algorithm, you use that iteration number as a reference to a normal object, mixed with a unique identifier.

This is all saved in a plain object {}.

So when you insert to DOM, you iterate through this object only ( register), and use the unique identifer on the node - if match with the register key - to perform the action you wanted?

I'm not sure if this is the way to go, but search "Bean" on github. You see there how they are using a internal register. That is not a VD approach, but you got the idea at least.

@dfilatov I thought of this a little more. I'm not used to VD, but maybe a better idea could be to store the functions on the prototype itself? I say you have a tag and text and a component?? Why not hook the function on here, so when you create a VD node you only have one prototype? You then wouldn't need to create many prototypes because the functions are already on the node.

Maybe can cause some overheads, so mix it with a register? Just my thoughts....

[dfilatov]

but maybe a better idea could be to store the functions on the prototype itself?

@rickardjanson Could you explain what you mean? The functions are already stored in the prototype.

[ghost]

@dfilatov Sorry. I was not clear. What I mean was on the first prototype. Let us say you create a text node. You need to construct the node. So on that prototype - same as the one for creating text - you can attach functions? In that case you should only need one prototype-

[ghost]

@dfilatov I got a much better idea here now that actually works. With prototype, and the render are down to 0.4ms on each update. No register, and only one prototype. 100 line of code.
Is there a place where I can send you a code snippet? Updated I say you had an email on your profile page, so just check your inbox.

I have no plans to develop a VD. This was just a working example. I may not reply on Github for some days, my works needs me. Have a nice day :)

[ghost]

@dfilatov I just dropped by to ask if you understood my ideas, and are they usefull for you? If so, I think I may have other ideas as well to gain better performance, if performance are important for you.