Skip to content

cache values and patterns in set_convert for rangebreaked axis #5659

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

Merged
merged 2 commits into from
Jun 23, 2021

Conversation

spasovski
Copy link
Contributor

@spasovski spasovski commented May 14, 2021

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
});
}


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

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
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
Copy link
Contributor

archmoj commented May 28, 2021

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
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
Copy link
Contributor

archmoj commented Jun 23, 2021

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