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

Optimize the text measuring of legend items when rendering large amount of traces #1586

Closed
wants to merge 2 commits into from

Conversation

hy9be
Copy link
Contributor

@hy9be hy9be commented Apr 11, 2017

I open this as a separate PR from #1585, because I am not sure if you guys will be willing to take this one.

The logic behind my change here is that if you want to plot reasonable amount of traces, we will give one very precise measuring and layout on legend items. But if you go crazy, we will sacrifice the precision of legend item layout (which probably does not matter anymore in this case) for way much better performance.

On my machine, this change reduced the rendering time for

  • 100 traces (200 points per trace) from 1.6 seconds to 1.1 seconds
  • 300 traces (200 points per trace) from 5.7 seconds to 2.8 seconds
  • 500 traces (200 points per trace) from 12.4 seconds to 5.8 seconds

@hy9be hy9be changed the title Optimize the text measuring logic for large amount of traces Optimize the text measuring of legend items when rendering large amount of traces Apr 11, 2017
@etpinard
Copy link
Contributor

My gut feeling would be to 👎 this PR. I don't think the perf improvement here is worth the maintenance burden / inconsistency.

Perhaps instead, we should make layout.showlegend default to false when there are more then say 100 traces on the graph in v2?

@hy9be
Copy link
Contributor Author

hy9be commented Apr 12, 2017

Okay. That makes sense.

@hy9be hy9be closed this Apr 12, 2017
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.

2 participants