Skip to content

Conversation

@benmccann
Copy link
Contributor

@benmccann benmccann commented Dec 31, 2019

I thought I'd share another update since I made a bit of progress since #6822. The main updates are:

  • Code reduction in constrollers.fastline
  • Tooltips are now supported
  • Added _getMinMax for increased performance

I'm not entirely sure the change here tooltip code would be worth adding. I think that if you really care about speed then implementing the tooltip externally would be the way to go. Though the change here would give the user the option and make it more fully featured compared to the normal line controller

The change to make setSyle static could be worth adding since it would save some code in implementing this vs copy-pasting the implementation

TODO: consider implementing fastpath

@kurkle
Copy link
Member

kurkle commented Dec 31, 2019

How does this compare to 'line'? (with animations disable etc)

@benmccann
Copy link
Contributor Author

benmccann commented Dec 31, 2019

On the uPlot benchmark (which has animations disabled):

With line:

  • Chart takes 550 ms
  • initialize takes 130 ms
  • _getMinMax takes 30 ms
  • updateDatasets takes 130 ms
  • draw takes 35 ms

With fastline:

The four affected methods are a total of about 4x faster.

It seems that object creation just really slows things down, which is why both this and #6791 are so impactful. One thing I noticed is the following code in updateElements creates a new object properties per data point:

const properties = {
	x,
	y,
	skip: isNaN(x) || isNaN(y)
};

if (includeOptions) {
	properties.options = me._resolveDataElementOptions(index, mode);
}

me._updateElement(point, index, properties, mode);

I don't fully understand all the new animation stuff, so am not sure if we could just set the properties directly on the Element instead of creating a new object? One way to do it if we need to defer setting the properties might be to do something like:

me._updateElement(point, index, function(element) {
    element.x = x;
    element.y = y;
    element.skip = isNaN(x) || isNaN(y);
    if (includeOptions) {
    	element.options = me._resolveDataElementOptions(index, mode);
    }
}, mode);

@benmccann benmccann force-pushed the fast-line branch 3 times, most recently from ef33420 to 357d856 Compare January 7, 2020 19:26
@benmccann benmccann force-pushed the fast-line branch 2 times, most recently from 99a74b0 to 27d65e6 Compare June 20, 2020 03:49
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