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

Improve performance by checking that the data changes #279

Merged
merged 3 commits into from
Feb 15, 2017

Conversation

PaulBGD
Copy link
Member

@PaulBGD PaulBGD commented Feb 6, 2017

Basically, anything that touches the DOM is slow. Because if that, this PR checks if any data changes before touching the DOM.

You can see the performance by comparing the dbmonster built with this PR to the normal one: https://svelte-dbmonster-perf.surge.sh/dist/index.html

Here are some screenshots from my late 2012 i5 macbook pro:

screen shot 2017-02-05 at 8 06 03 pm
screen shot 2017-02-05 at 8 06 14 pm

The dbmonster based off this PR will jump up to 600fps, but the regular dbmonster doesn't go past 60fps. I'm not sure why the dbmonster based off this PR has such sudden fps jumps (although it doesn't drop below 60 for me.)

@PaulBGD
Copy link
Member Author

PaulBGD commented Feb 6, 2017

After testing this in chrome, it seems that this only has such a big performance improvement for Firefox. I guess chrome must internally check if it changed and firefox doesn't.

@Swatinem
Copy link
Member

Swatinem commented Feb 8, 2017

Depending on the expression, snippet might be pretty long (codesize-wise) and also be slow (think computed accessors).

I was thinking about creating a helper for text nodes (and later also for attributes, see #25 ) that does memoization and comparison internally.
That textnode helper might have the same api as fragments, with update, mount, and detach functions.
What do you think about that?

@Rich-Harris
Copy link
Member

Wow, that's an impressive speedup on Firefox. Memoizing helpers is definitely worth exploring — there might be an overhead involved (truthfully I have no idea) so maybe the right move in the meantime would be to assign ${snippet} to a tmp variable (which could be reused) and comparing that to last_${name}?

@PaulBGD
Copy link
Member Author

PaulBGD commented Feb 8, 2017

Good idea about assigning ${snippet} to a temporary variable.

@PaulBGD
Copy link
Member Author

PaulBGD commented Feb 8, 2017

So since I'm finally on a windows computer I decided to try this on Edge.

Regular:
Image

PR:
Image

The FPS isn't very different but the change between view update time is huge.

@Rich-Harris Rich-Harris merged commit 3bcdab9 into sveltejs:master Feb 15, 2017
@Rich-Harris
Copy link
Member

I implemented the temporary variable thing, and extended this to {{{triples}}} and attributes

@PaulBGD
Copy link
Member Author

PaulBGD commented Feb 15, 2017

Looks good.

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

Successfully merging this pull request may close these issues.

None yet

3 participants