Skip to content
This repository has been archived by the owner on Jul 29, 2019. It is now read-only.

Timeline: Performance issues #3248

Closed
mojoaxel opened this issue Jul 11, 2017 · 12 comments
Closed

Timeline: Performance issues #3248

mojoaxel opened this issue Jul 11, 2017 · 12 comments
Assignees

Comments

@mojoaxel
Copy link
Member

I noticed some performance issues with groups.

I created a demo with 20 groups an 400 items:
image

Source can be found on jsbin.

The rendering takes about 2 seconds:
image

The main problem is, that every RangeItem.redraw triggers a DOM reflow:
image

Maybe it is possible to redraw the items all at once?

@mojoaxel mojoaxel added this to the Minor Release v4.21 milestone Jul 11, 2017
@grimalschi
Copy link
Contributor

It looks like question is not about redraw items all at once.

Problem is that we read properties, then we write properties, then we read properties again, and browser make reflow, then we write, read, then reflow again.

I think, the way to fix this is to separate reading attributes and writing.

Same as this library does: https://github.com/wilsonpage/fastdom

@grimalschi
Copy link
Contributor

And actually I dont understand why we dont have such a problem with no groups.

If we have no groups, but a lot of items, timeline works good.

Maybe there is special read/write properties operation in group redraw.

@grimalschi
Copy link
Contributor

@mojoaxel i have some success here.

This is performance before changes (from my app):

image

This is performance after changes:
image

About 5 times increased performance :)

@grimalschi
Copy link
Contributor

In your case I have only 2 times performance increasing.

Before:
image

After:
image

Demo: https://jsbin.com/bibehifufu/1/edit?html,js,output

@grimalschi
Copy link
Contributor

This is what I did: https://gist.github.com/grimalschi/df9a44ac12b5420e0cc6f2d42f1febed/revisions

Can we use this somehow?

@mojoaxel
Copy link
Member Author

This is what I did: https://gist.github.com/grimalschi/df9a44ac12b5420e0cc6f2d42f1febed/revisions
Can we use this somehow?

@grimalschi Thank you very much for debugging this!! It looks promising.

I hope @yotamberk can have a look at this and maybe even provide a fix. He implemented the groups as far as I remember.

@yotamberk
Copy link
Contributor

I'll be on it next week after I finish my exams.

@yotamberk
Copy link
Contributor

@grimalschi I'm going over your revisions and don't quite get how putting all the actions in to a queue array solves the "read, write, read, write..." problem. Can you please explain?

@grimalschi
Copy link
Contributor

@yotamberk I took idea here: https://github.com/wilsonpage/fastdom

When you write new properties, then browser can just note that this element position is unknown. And then if you read some properties, browsers looks at the note and makes reflow to know element position and to return it.

So if you do write and then you do read, you have reflow. If you do cycle "write, read" 100 times, you have 100 reflows. As we have on screenshots.

And if you change "write, read" 100 times to "write" 100 times and "read" 100 times, then you have only 1 reflow.

Because you do 100 operations and you browser don't need to make reflow.

So what I did is just changed this queue:
write element 1
read element 1
write element 2
read element 2
to this:
write element 1
write element 2
read element 1
read element 2

I did it bit ugly way, but if you understand idea, you can do it more clean.

Also you can read explanations for this: https://github.com/wilsonpage/fastdom

@wimrijnders
Copy link
Contributor

@yotamberk I think you tackled this issue with #3409 and #3475. Can the Work in Progress label be removed now? And a Fixed Awaiting Release set?

@mojoaxel
Copy link
Member Author

Looks like this problem was fixes!
@yotamberk Thanks so much!

The version using 4.21.0 is much faster: https://output.jsbin.com/gitawoyege

@montkari
Copy link

Hello @yotamberk and @mojoaxel, thank you very much for the support, correction of this problem and for the great strength that exists in the community. You have been helping a lot of people, Congratulations

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

No branches or pull requests

5 participants