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

Large speed up and benchmark for graphs with many points #896

Merged
merged 1 commit into from
Dec 31, 2017

Conversation

vii
Copy link
Contributor

@vii vii commented Dec 11, 2017

Adding fields to a structure after it is created can be slow on some
browsers, like Chrome.

Adding a demo to benchmark many points, this seems to speed up my
Chromium from about 2.8-3s to about 1.8s.

Thanks to Christopher Palmer (@thecav) for the analysis and specific
suggestion.

…e start

Adding fields to a structure after it is created can be slow on some
browsers, like Chrome.

Adding a demo to benchmark many points, this seems to speed up my
Chromium from about 2.8-3s to about 1.8s.

Thanks to Christopher Palmer (@thecav) for the analysis and specific
suggestion.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 90.174% when pulling eec9dae on vii:master into 8dc4700 on danvk:master.

@vii vii changed the title Make sure that canvasx and canvasy properties are initialized from the start Large speed up and benchmark for graphs with many points Dec 16, 2017
@vii
Copy link
Contributor Author

vii commented Dec 30, 2017

@danvk would love some feedback! @saboya has some improvements upon the optimizations

@danvk danvk self-assigned this Dec 31, 2017
@danvk
Copy link
Owner

danvk commented Dec 31, 2017

Thanks for this change @vii. I can confirm that it is also a speedup for me in Chrome 62.

I don't think a performance benchmark is appropriate for the gallery, so I'm going to revert that change. Have you seen tests/dygraph-many-points-benchmark.html? It does something similar.

@danvk danvk merged commit 61767ef into danvk:master Dec 31, 2017
@danvk
Copy link
Owner

danvk commented Dec 31, 2017

I'll hold off on cutting a new release until @saboya sends a PR.

@saboya
Copy link

saboya commented Dec 31, 2017

Hello.

The link that I made on my commit was actually just a reference, it's not an improvement over @vii 's code, it's just an implementation using a custom dataHandler on my fork of react-dygraphs. Since dygraphs releases usually take some time, I just did that in order to benefit from it right now.

Thanks for your work on dygraphs, and happy new years everyone :)

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.

4 participants