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

Performance improvements for lineChart #1282

Merged
merged 5 commits into from Oct 3, 2015
Merged

Conversation

tony-jacobs
Copy link

When lineChart was unified with lineWithFocusChart, a bug crept in cause the main chart to be called twice (once during the standard update, again during the onBrush() update). This patch makes those two updates to the main chart mutually exclusive, based on the focusEnable flag.

The majority of the diff is whitespace, now that the focusChart logic is wrapped by an if.

A few other changes are along for the ride:

  • The background rects are now added with the rest of the skeleton code instead of (confusingly) later in the code.
  • Avoid creating a local variable called 'that' on general principal. It was only used in one place.
  • The scatter model now offers an option for the interactiveUpdate duration - it had been hard coded to 300ms. On my machine, that deferred update was taking 8ms to set up while a direct update was completed in under a millisecond. Library users can now choose to accept a degraded animation in favor of faster raw chart updates. Note: I wasn't able to visually spot a difference from running at the default 300ms, so perhaps it's safe to set the default on that to 0?

Tony Jacobs added 4 commits October 2, 2015 23:42
…of a setTimeout in performance-critical code path.
…tead of doing an additional select later on.
…rush when focus is not enabled. Also, avoid pre-evaluating main line chart when focus is enabled, since the focus chart may filter.
timeoutID = setTimeout(updateInteractiveLayer, interactiveUpdateDelay );
}
else
updateInteractiveLayer();
Copy link
Contributor

Choose a reason for hiding this comment

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

Please always use brackets

@tony-jacobs
Copy link
Author

Added documentation and braces per comments.

liquidpele added a commit that referenced this pull request Oct 3, 2015
Performance improvements for lineChart
@liquidpele liquidpele merged commit b255bca into novus:master Oct 3, 2015
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.

None yet

2 participants