Skip to content

Address scattergl performance issue with TypeArrays - #5632

Merged
archmoj merged 2 commits into
plotly:masterfrom
Seranicio:master
May 5, 2021
Merged

archmoj merged 2 commits into
plotly:masterfrom
Seranicio:master

Conversation

@Seranicio

@Seranicio Seranicio commented May 5, 2021

Copy link
Copy Markdown

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
Comment thread src/components/color/index.js Outdated
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
Copy Markdown
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

archmoj commented May 5, 2021

Copy link
Copy Markdown
Contributor

Thanks very much for the PR.
LGTM.

@alexcjohnson alexcjohnson left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

💃

@nicolaskruchten

Copy link
Copy Markdown
Contributor

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

@archmoj

archmoj commented May 5, 2021

Copy link
Copy Markdown
Contributor

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