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

Speed up the rendering process #2034

Closed
GianlucaGuarini opened this issue Oct 19, 2016 · 7 comments
Closed

Speed up the rendering process #2034

GianlucaGuarini opened this issue Oct 19, 2016 · 7 comments

Comments

@GianlucaGuarini
Copy link
Member

GianlucaGuarini commented Oct 19, 2016

According to my tests riot is too slow during the rendering and mounting process

All tests were done on Firefox 54

@GianlucaGuarini
Copy link
Member Author

GianlucaGuarini commented Oct 19, 2016

Btw riot@next appears to be twice faster than the riot@2 but I am still not 100% happy

@vitogit
Copy link
Contributor

vitogit commented Oct 20, 2016

Are you using the chrome devtool profiler? It could be useful to know where most time is spent https://i.imgsafe.org/8189f62f89.jpg

@rogueg
Copy link
Contributor

rogueg commented Oct 20, 2016

Your test uses an each, which creates a new (anonymous) tag for every item in the list. Creating tags in riot is pretty slow, because it does so much:

  • Create new DOM using setInnerHtml (which the browser then has to parse)
  • Walk that DOM and parse expressions (regex against every attribute and text node)
  • Evaluate those expressions and update the DOM

A while back I was kicking around the idea of "tag stamping" to avoid some of the redundant work here. Basically you'd do setInnerHTML and parseExpressions just once, to create a "pristine" version of the tag. Every time you wanted an instance, you'd cloneNode and copy the expressions.

It's a pretty major change though, and I didn't really want to work on it until riot 3 was out.

@GianlucaGuarini
Copy link
Member Author

GianlucaGuarini commented Oct 21, 2016

Thanks @rogueg for your answer, I have also detected the parseExpressions as the origin of the issue. I think we could solve the issue caching and rebinding the expressions like you said in a sort of virtual tree for each of the looped nodes. The question is more if we should really implement this sort of virtual dom also for the other tags as well. Another question is, how do we parse the expressions injected via yield then?

It's a pretty major change though, and I didn't really want to work on it until riot 3 was out.

@rogueg I think we could already work on it for the following reasons:

  1. The compiler is not ready yet and probably will require still a bit of time ( @aMarCruz could tell us more about it )
  2. I would prefer to make a release that solves all the performance issue riot had before, I was focused on the update method that was heavily improved, but the mount should be improved as well
  3. This update is not a breaking change because it will change how riot handles the DOM creation internally so it has no side effects for our users

@irisjae
Copy link

irisjae commented May 29, 2017

Are the numbers in the OP the latest numbers?

@aMarCruz
Copy link
Contributor

aMarCruz commented Jun 13, 2017

I think the compiler will took ~4 weeks.

The tags can be instances of a real prototype created by the compiler.

The compiler must emit (almost) pure JS-precompiled code as low-level instructions to the render machine (pluggable?), see (this comment)[https://github.com//issues/2283#issuecomment-308052204] and note the riot._render.tag().

I'm not sure yet where to get data , maybe from this.state or from the closure of the render method.

@GianlucaGuarini
Copy link
Member Author

the riot loops rendering was heavily improved and the tests above show almost equivalent values across all the libraries except for polymer that has a really slow boot. I am closing this issue moving forward

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

No branches or pull requests

5 participants