Skip to content

cache values and patterns in set_convert for rangebreaked axis - #5659

Merged
archmoj merged 2 commits into
plotly:masterfrom
spasovski:perf-issue
Jun 23, 2021
Merged

cache values and patterns in set_convert for rangebreaked axis#5659
archmoj merged 2 commits into
plotly:masterfrom
spasovski:perf-issue

Conversation

@spasovski

@spasovski spasovski commented May 14, 2021

Copy link
Copy Markdown
Contributor

Fixes #5630

set_convert could benefit from caching the simpleMap results. My team and I experienced drastic performance issues on ticking plots with rangebreaks. We are quite open to better suggestions though this approach worked quite well for us in testing.

@spasovski spasovski changed the title Perf issue cache values and patterns in set_convert for rangebreaked axis May 14, 2021
Comment thread src/plots/cartesian/set_convert.js Outdated


for(var i = 0; i < rangebreaksIn.length; i++) {
var brk = rangebreaksIn[i];

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would cache against each brk group, instead of the rangebreaks array as whole, lets you get rid of the nulls and puts the cached values immediately adjacent to the raw values.

@spasovski
spasovski requested a review from archmoj May 21, 2021 18:37
@spasovski

Copy link
Copy Markdown
Contributor Author

@archmoj please let me know your feedback on this, would love to know if the approach is good or if it needs changes

@archmoj

archmoj commented May 28, 2021

Copy link
Copy Markdown
Contributor

Thanks @spasovski.
It looks like something we could possibly include in the v2.0.1 patch cycle.
I'll try to get back to you on this a bit later (potentially next week) right after the v2 is out.

@spasovski

Copy link
Copy Markdown
Contributor Author

Hi folks please let me know if this approach seems solid. We've been using this in our staging environment for a few weeks and haven't had any issues so far.

@archmoj

archmoj commented Jun 23, 2021

Copy link
Copy Markdown
Contributor

Looking good to me.
Thanks very much for the PR.
💃

@archmoj
archmoj merged commit 608337e into plotly:master Jun 23, 2021
@archmoj archmoj added this to the NEXT milestone Jun 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community community contribution

Projects

None yet

Development

Successfully merging this pull request may close these issues.

poor performance of chart layout when data includes rangebreaks

3 participants