-
Notifications
You must be signed in to change notification settings - Fork 12k
Remove ticksToTimestamps and change ticks to numbers #6791
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
|
Did not review yet, but in general I think I think there is something to fix elsewhere. Some generic thoughts:
|
|
Simon had suggested the current approach of making
It sounds like basically we would want that to result in essentially the same outcome as One thing I don't like right now is how much both the core scale's |
e370ad7 to
2ec4eb2
Compare
|
@kurkle any thoughts on this? I can't really come up with any other way to improve performance here |
kurkle
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of notes. Lets keep this open for a while still, I haven't had the time to take a look in depth yet.
This could be the way to go with timeSeries scale, if splitting them up makes sense. For linear mode I think it is going to be more efficient to generate the labels from min/max only.
572b8c4 to
9167445
Compare
ticksFromTimestampsis pretty expensive in the uPlot benchmark since it creates an object for every tick. Leaving ticks as primitives is much more performant and will be one less thing for the user to migrate since that's how it is in 2.9 for all scales other than the time scale. This now stores themajorattribute in a separate_tickMetaarray of objects, which is much cheaper since an object only needs to be created whenmajor: truebuildTickstimingBefore: 73.05 ms
After: 19.23 ms
This makes the total render time for the Chart about 10% faster