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

Address scattergl performance issue with TypeArrays #5632

Merged
merged 2 commits into from
May 5, 2021

Conversation

Seranicio
Copy link

@Seranicio Seranicio commented May 5, 2021

Address: #4401

Overview of the Pull Request:

Address an unavoidable loop phase inside color clean function where
it iterates all over the graph points if data used is a derivative of a ArrayBuffer.
This causes graph to have way less performance relative to Array types.

Also added a little performance enhancement to calc function. Tell if this is out of scope :)

Simple Demo: here (adapted from @archmoj)

Function performance checkup:

Screenshot 2021-05-05 at 12 47 00

Address an unavoidable loop phase inside color clean function where
it iterates all over the graph points if data used is an ArrayBuffer.
This causes graph to have way less performance relative to Array types.
@archmoj archmoj added status: reviewable community community contribution labels May 5, 2021
@@ -116,7 +116,7 @@ color.clean = function(container) {
if(!Array.isArray(el0) && el0 && typeof el0 === 'object') {
for(j = 0; j < val.length; j++) color.clean(val[j]);
}
} else if(val && typeof val === 'object') color.clean(val);
} else if(val && typeof val === 'object' && !ArrayBuffer.isView(val)) color.clean(val);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's only allow typed arrays here by using the isTypedArray function.

function isTypedArray(a) {
return ab.isView(a) && !(a instanceof dv);
}
exports.isTypedArray = isTypedArray;

To use that you could require it like this:

var isTypedArray = require('../../lib/array').isTypedArray;

@Seranicio Seranicio changed the title Address performance issue with ArrayBuffers Address performance issue with TypeArrays May 5, 2021
@archmoj archmoj changed the title Address performance issue with TypeArrays Address scattergl performance issue with TypeArrays May 5, 2021
@archmoj archmoj added this to the NEXT milestone May 5, 2021
@archmoj
Copy link
Contributor

archmoj commented May 5, 2021

Thanks very much for the PR.
LGTM.

Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💃

@nicolaskruchten
Copy link
Contributor

Does this PR fully resolve #4401 i.e. the part about line-decimation or just the part about Arrays?

@archmoj
Copy link
Contributor

archmoj commented May 5, 2021

Does this PR fully resolve #4401 i.e. the part about line-decimation or just the part about Arrays?

No. I reopened the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community community contribution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants