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

Change generateData() to preserve object identity. #19

Closed
mhevery opened this issue Sep 16, 2015 · 10 comments
Closed

Change generateData() to preserve object identity. #19

mhevery opened this issue Sep 16, 2015 · 10 comments

Comments

@mhevery
Copy link
Contributor

mhevery commented Sep 16, 2015

I have only looked at Angular2 and t7.

The two frameworks make different assumptions on how to interpret the change in data and as a result mutate the DOM in very different way, which then results in a material performance difference.

The mutation of the data happens in generateData() method. The generateData returns a new array with new items in it each time it runs. This has important implication. Because the rows in the returned array have different identities, there are two ways of interpreting the data:

  1. Ignore the identity and assume that first item in array is same as first item in the old array. This is what t7 does. Implication is that we just have to update the existing DOM structure with the new values and we are done.
  2. Look at old/new array and compute how the identities changed. This is what Angular 1 and 2 do. In this case the change is interpreted as all items left the array and new set of items were inserted. Angular will try to honor the removal/inserts and so it will remove all DOM rows for the items which were removed and then insert a new set of DOM rows for the new items back in. Angular 2 can do this relatively efficiently, because we can cache and reuse the rows.

Both strategies, look reasonable at first, until you try to repeat over user input elements, or try do do animation, then strategy 1 breaks.

  • Animation: reusing the row will not animate old row away and new one in. At best it could animate new rows additions or extra rows removal. The result is that inserting a new item in the front of the array will result in data jumping to next row (identities are not checked) and the last row is animated in. This does not reflect what actually happens in the model.
  • User Input: If the user is editing a form which has repeated rows, inserting a new row in the front will clobber all values including the user cursor and or selection.

Because of the above corner cases which are hard to debug (insertions at end works, but insertion in the middle does not, but only if user is typing and or animations are enabled), Angular uses the second strategy. In most real life applications this is not an issue, since it is very rare that the model is constantly changing the identity. In real life cases model either does not change or only changes values. If the model only changed values but not the object identity, then Angular would not delete/insert rows and the performance would be identical.

Which brings are to the last use case of track-by. There are cases where you want Angular to treat two objects with different identities as equivalent. For example calling the backend and getting a refreshed dataset from server would appear as new objects which would then animate the existing data away and animate new data in. In such a case you can specify a track by function, which allows Angular to recognize that two objects should be treated as equivalent and reuse the corresponding rows. This is why Angular 1 with track by performs better then Angular 2 without track by, because the track by causes Angular 1 to interpret the data in the #1 way. BTW, track-by is coming to Angular 2 as well, we just have not gotten to it.

The summary is that these perf tests do not compare apples to apples. Because generateData() method creates new data set each time, it is hard to say which strategy individual frameworks have chosen to implement when doing their updates. My suggestion would be to update generateData() method so that it updates the data in a way which keeps the identities same. This is a much more realistic case, in which case all of the frameworks will interpret the change in a same way.

@mathieuancelin
Copy link
Owner

Hi @mhevery

I've changed the way data is generated, and indeed, angular performs way better with.

But I'm not sure it's something I want to push because, it's an optimization for Angular as angular is tracking object by their identities.

The idea of this project is to provide naïve implementations (I'm not sure every implementation is naïve, but I hope so). Keeping row identities is not a real world use case, data returned by an REST service will never keep row identities, and more important, it's not how a new user will use Angular.

But you can provide optimized example (in the optimized version section) with identities and track by strategy when it's implemented.

I'll try to push the new data generator asap, with a flag to turn identity keeping strategy on.

@mhevery
Copy link
Contributor Author

mhevery commented Sep 17, 2015

On Thu, Sep 17, 2015 at 3:09 AM, Mathieu ANCELIN notifications@github.com
wrote:

Hi @mhevery https://github.com/mhevery

I've changed the way data is generated, and indeed, angular performs way
better with.

Would love to see the numbers

But I'm not sure it's something I want to push because, it's an
optimization for Angular as angular is tracking object by their identities.

So tracking identities is way harder (in terms of code complexity as well
as CPU time) then assuming that array order is same. So Angular is trying
to be more correct here so we can properly do animation and user
intercations. We could change Angular to be positional by default and you
would have to opt-into the track by identities, but it feels wrong to me to
do it just to satisfy the benchmarks.

From my point of view the other frameworks are buggy in this behavior. This
is a situation where doing nothing/less is actually the wrong behavior.

The idea of this project is to provide naïve implementations (I'm not sure
every implementation is naïve, but I hope so). Keeping row identities is
not a real world use case, data returned by an REST service will never keep
row identities, and more important, it's not how a new user will use
Angular.

Correct REST use case is a good example, and there track-by should be used,
but in case of REST, the REST call outweighs the cost of render by orders
of magnitudes, so I am not sure it makes a material difference. I don't see
a use case where the rest calls are coming in at 50 fps and we need the UI
to keep up with them. Any changes to model at 50 fps rate are timer driven
not REST driven.

But you can provide optimized example (in the optimized version section)

with identities and track by strategy when it's implemented.

That is our plan.

We are having a lot of discussion internally about this. Should we do what
is fast, even if it is sometimes wrong, or should we do what is correct by
default. Do you have thoughts?

I'll try to push the new data generator asap, with a flag to turn identity
keeping strategy on.

That would be awesome.


Reply to this email directly or view it on GitHub
#19 (comment)
.

@mathieuancelin
Copy link
Owner

Hi @mhevery,

I've pushed a new data generator with same identities for rows (I hope the code is right, if you can review it)

749e1df

also I've added optimized Angular 1 & 2 version here

30010aa

http://mathieuancelin.github.io/js-repaint-perfs/angular/opt.html
http://mathieuancelin.github.io/js-repaint-perfs/angular2/opt.html

@mhevery
Copy link
Contributor Author

mhevery commented Sep 18, 2015

Hi @mathieuancelin,

The numbers look a lot better, but it is not the whole story. Currently c7 and Angular2 test case are limited by the browser. In both cases the rendering dominates, and browser will not render faster then 60 fps. So what you are measuring now is that both tests complete faster the browser can render, it does not actually answer the question of which one is faster.

(These charts are done using Web Tracing Framework)

Here is what it looks like for C7:
screen shot 2015-09-18 at 9 31 52 am

The green bars are frames. The purple large bar is the setTimeout which updates the DOM and the blue short bar is the request animation frame which updates the fps rate in the corner of the screen. Notice that the a large amount of white between the bars. That is the browser doing rendering. It is what limits your speed. Even if the purple bars would get smaller the whites are already dominating.

On average it took 7.148ms to execute the purple bar, which is the generate() plus DOM update. (some bars are bigger due to GC)

Now lets look at Angular 2:
screen shot 2015-09-18 at 9 37 54 am

NOTE: the zoom level is different here, so you can't look at size of the bars, but the green bars are frames and they are still at 60fps, so use green bars as mile markers.

Same story here. The browser is the limiting factor.

An average it took 9.55ms to update the purple bar. So Angular 2 is slightly slower the C7 in this test. Let's have a look why. I will zoom into one of the purple bars. (NOTE: Angular hooks into WTF and the calls are not free which adds about 0.005 ms per call (600 calls) which in this case adds up to about 3ms. So if you subtract 3ms from 9.5ms you get about 6ms, which means Angular should be slightly faster then c7 with WTF off, but let's ignore this for a second)

screen shot 2015-09-18 at 9 42 57 am

In the above graph what we see is that the purple setTimeout is broken into two parts.

1.) the purple bar with nothing below it. This is your call to generate() when I look into measurements I see that takes 1ms.
2.) the purple bar with green LifeCycle#tick bar, which is where the Angular updates happens. Ideally it should be just pure cyan color below it, which means Angular has looked at data, it saw it change and it update DOM as needed with no structural changes. What I see instead is that there are lots of green and purple things below it. When I zoom into them, I still see that that Views are getting created and destroyed.

My explanation is that the outer ng-for is stable, but the inner ng-for is still looking at data which does not have stable identity. (Yes, this will be fixed with track-by but for now it means that your genrate() method still creates data churn)

If you fix the second identity churn on inner ng-for you will find that Angular2 will be faster then c7, not that you will be able to tell the difference since it will be limited by the browser anyways.

@mathieuancelin
Copy link
Owner

Hi @mhevery,

the last commit should do the trick :

b86281e#diff-d843cdb1260fcc6789effbb70eb0a2bfR96

but I can't really try it as I can't reach 60 fps on my machine :-)

@mhevery
Copy link
Contributor Author

mhevery commented Sep 21, 2015

Nice work sir! It is way faster! Here is what I am seeing.

WTF: Angular 2

screen shot 2015-09-21 at 11 16 24 am

I see pure cyan color! this is great. It means that we are doing no structural changes, only DOM update.

I get 3.67ms on each update This breaks down to 1.9ms for Angular which leaves 1.77ms for the generate() method. But this is with WTF on, so some of it is WTF. When we look at sampling in timeline we se that the true update is closer to 1.5ms.

screen shot 2015-09-21 at 11 24 07 am

So out of 3.8ms which is what it takes for the script to run (without WTF) Angular DOM update is 1.5 ms which leaves 2.3ms for the generate() method. So the two methods of measurement do average about 2ms depending on how you measure.

The point is that your application code is outweighing the test code, and as a result is dominating the time. Which means you are not measuring what you think you are measuring.

WTF: t7

screen shot 2015-09-21 at 11 15 18 am

Shows that on average the update is 4.5ms. Already this is slower then Angular 2 which was 3.8ms. But remember that 2ms is generate which leaves t7 to do update in 2.8ms.

So at the end we have c7 at 2.8ms and Angular 2 at 1.5ms. So Angular2 is 50% faster in this case. Now this is hard to see since at the setTimeout level which includes generate the speed up is only 15%. Once you throw painting into the mix the speed up is not that dramatic.

Never the less. I get 80 fps on Angular 2 and 60 fps on t7 on my machine

But the story is not done. Let's look at GC pressure.

Angular2:
screen shot 2015-09-21 at 11 35 14 am

1.08s between GC with 40MB going up to 48MB - GC - 40MB or 8MB per GC cycle. or a rate of 7.4mb per second.

t7:
screen shot 2015-09-21 at 11 35 05 am

2.85ms between GC with 18MB going up to 107MB -GC- 16MB or 90MB per GC or 32MB per second.

As you can see Angular app generates lot less garbage and as a result the GC can run often with very little pause and memory pressure.

@mathieuancelin
Copy link
Owner

That's great !

It's a shame that I can't reach the top frame rate, on my machine Angular 2 is rendering a 30/35 fps max according to Chrome Dev Tools.

Do you think we can do something else to globally enhance the test ?

@mhevery
Copy link
Contributor Author

mhevery commented Sep 22, 2015

Do you see the same ratios between t7 and angular2 as I do?

We could make the generate function faster so that the difference in
frameworks could better come up.

We could play with CSS so that it is easier for browser to paint.

On Tuesday, September 22, 2015, Mathieu ANCELIN notifications@github.com
wrote:

That's great !

It's a shame that I can't reach the top frame rate, on my machine Angular
2 is rendering a 30/35 fps max according to Chrome Dev Tools.

Do you think we can do something else to globally enhance the test ?


Reply to this email directly or view it on GitHub
#19 (comment)
.

@mathieuancelin
Copy link
Owner

I've got something like :

  • 37 fps for Angular 2
  • 34 fps for t7

Faster data generation would be great, if you have some ideas, please don't hesitate.

@mhevery
Copy link
Contributor Author

mhevery commented Sep 22, 2015

Instead of doing complex loops and math.random, just have one function
which sets the data and alternate between a and b value.

On Tue, Sep 22, 2015 at 11:45 AM, Mathieu ANCELIN notifications@github.com
wrote:

I've got something like :

  • 37 fps for Angular 2
  • 34 fps for t7

Faster data generation would be great, if you have some ideas, please
don't hesitate.


Reply to this email directly or view it on GitHub
#19 (comment)
.

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

No branches or pull requests

2 participants