determineDataLimits optimizations

[kurkle]

• Math.sign replaced with helpers.sign, for IE compatibility
• ticks.min / ticks.max parsed at scale creation for early access.
• _getOtherScale internal helper
• controller._getMinMax: skip values limited out of chart by other scale
• scale._getMinMax: skip loops if valid min & max are defined
• scale.category: use _parsedMin / _parsedMax in determineDataLimits
• scale.logarithmic: rename nonNegativeOrDefault to positiveOrDefault, better minimization

Resolves: #4131
Pen

[benmccann]

This looks good to me. I think there's a lot more we can do to optimize if we update the format that we accept data in. I started a discussion on that in a new issue #6696

[kurkle]

Rebased + added some optimizations to time scale. Removal of duplicate arrayUnique when ticks come from only one source (data / labels) in getAllTimestamps is noticeable (somewhere between 5-10% in uPlot bench).

[benmccann]

We could end up doing arrayUnique and sort three times. Once in getDataTimestamps, once in getLabelTimestamps`, and once here. I think it'd be preferable if we could make it work more like it did before the early parse PR where it could happen only once

[kurkle]

I wonder why the uPlot benchmark was done with distribution: 'series'
If time is plotted linearly (default), we can get away without any arrayUnique calls.

[leeoniya]

probably because your timeseries demo uses 'series' :D

https://www.chartjs.org/samples/latest/scales/time/financial.html

[kurkle]

Seems it is done for demonstrating weekend exclusion from the chart.

[leeoniya]

interesting. something for me to think about for https://leeoniya.github.io/uPlot/demos/high-low-bands.html

[benmccann]

We should also be able to get rid of arrayUnique if we allow passing in the data as a single dataset instead of making the user specify the timestamps again for each line #6696

[kurkle]

That can already be done by using labels and (primitive) data, and ticks.source='labels'.
Should try out the new data format(s), how it would work out.

[kurkle]

New uPlot benchmarks:

master - series: ~850ms
master - linear: ~650ms
2.9.2 - series: ~700ms
2.9.2 - linear: ~550ms
pr - series: ~700ms
pr - linear: ~450ms

[benmccann]

We have always assumed that data is in order and unique for a single dataset. I'm not sure I want to change that. Both because I don't know what other code has those assumptions built in and also because it'd be worse from a performance point of view

[kurkle]

Not always, this assumption was made in quite recent optimizations. Originally arrayUnique was called regardless of number of datasets.

I can think of use cases where timestamps would not be in order (and should be drawn that way). But those are not the "usual" cases, so maybe it should be behind a switch.

I think this would be a lot more maintainable, if "series" and "linear" time scales were separated. "linear" could be based mostly on LinearBase.

[benmccann]

I think I agree that it'd be more maintainable to have "series" and "linear" as separate scales. There's a lot of complication trying to cram both concepts into a single scale. I'm not sure there's a lot you could share between the linear time scale and linear scale. There is a lot you could share between the linear and series time scales though. Not sure whether it'd be better to have one extend the other or make a base time scale

[kurkle]

I'll explore that in a later PR

[benmccann]

a couple small thoughts

[benmccann]

@kurkle the build failed

[kurkle]

The improved (only visible data is accounted for) min/max determination changed scale ranges on some of those clip fixtures. Updated the images.