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 Chart.js benchmark by skipping label rotation #25

Merged
merged 1 commit into from
Oct 21, 2019

Conversation

benmccann
Copy link
Contributor

I'm working on the Chart.js documentation to add some performance tips and have suggested they add this tip. I don't think Chart.js will ever be as fast as uPlot, but it should be at least as fast as Highcharts, etc.

Setting maxRotation: 0 allows Chart.js to skip calculating how much to rotate the labels to fit them on the chart and just makes them all horizontal.

autoSkipPadding mostly just makes it look a little nicer by putting some spacing between labels

@leeoniya
Copy link
Owner

hey @benmccann, thanks for the PR.

i can merge this but the net effect (if there is one) appears to be noise-level. not all that surprising since so few labels need to be measured here.

are you seeing anything different on your end?

@benmccann
Copy link
Contributor Author

Ah, yeah, it looks like you're right. I was working off a locally build version of Chart.js to debug perf issues and it looks like that version is taking longer for some reason

If you go back to Chart.js 2.6.0 this benchmark runs a lot faster. I've been trying to track down the regressions. I think we fixed the main one, so hopefully Chart.js 2.9.0 won't be quite as terrible

@leeoniya
Copy link
Owner

do you want to hold off and PR this with 2.9 then?

@benmccann
Copy link
Contributor Author

Whatever's easier for you. I don't think this option hurts and would probably specify it either way. At some point in the next couple weeks I'll follow up to bump the version number

@leeoniya leeoniya merged commit a43356c into leeoniya:master Oct 21, 2019
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