-
Notifications
You must be signed in to change notification settings - Fork 12k
Perf improvement for ticks.source:'labels' #6354
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
Conversation
|
What about Another thing to consider is the way labels are implemented. Those might as well be intended for a category scale in a time/category chart. So we need the check if its object data or not. Then we could have a mixed set of datasets with object data and plain data. So would need to check if any of those datasets have plain data and add labels then. I'd maybe add a There is also a bug (in master), we are not checking if dataset is any of current scales business. |
|
Thanks for the great review @kurkle. I've made the changes you suggested |
* Perf improvement for ticks.source:'labels' * Address review comments * Address review comments
We only need to add the
labelstotimestampsonce - not once per datasetWe also don't need to call
arrayUniqueon the labels because duplicate labels aren't supported. If you pass duplicate labels and then we callarrayUniquethen we end up with fewer labels than data points, so this code never worked.arrayUniqueis quite expensive for large datasets, so it's very helpful to remove. And then we no longer need to callsortwhich is also expensive. That was done only becausearrayUniquemay not return items in the order they were passed in