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

Improve performance when transmission lines are not drawn #1153

Merged
merged 2 commits into from
Jun 3, 2020

Conversation

jameshadfield
Copy link
Member

#1147 implemented a toggle for transmission lines. This works by setting the stroke-width for the lines to 0, so they are still calculated and placed in the DOM. There is significant overhead to both the calculation and the DOM manipulation for these lines, especially noticeable during animation.

This PR skips the creation of transmissionData and transmissionIndices when we are not displaying transmission lines, and the corresponding DOM elements are never created.

Performance testing (locally using Firefox dev-tools, displaying only the map panel, using a nCoV dataset with 4.3k tips) shows a ~50% increase in framerate (8.8 -> 12.7fps).

This PR is a single commit (a9ef44b) on top of PR #1152

This skips the creation of `transmissionData` and `transmissionIndices` when we are not displaying transmission lines, and the corresponding DOM elements are never created.
@jameshadfield jameshadfield requested a review from eharkins June 3, 2020 06:04
@jameshadfield jameshadfield temporarily deployed to auspice-transmission-pe-soqcto June 3, 2020 06:05 Inactive
@jameshadfield jameshadfield changed the title Transmission perf Improve performance when transmission lines are not drawn Jun 3, 2020
Copy link
Contributor

@eharkins eharkins left a comment

Choose a reason for hiding this comment

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

Looks good. Didn't measure performance just checked that it was indeed not computing transmission lines when not displaying them.

@@ -37,6 +37,7 @@ For instance, if you set `display_defaults.color_by` to `country`, but load the
| `layout` | Tree layout | "rect", "radial", "clock" or "unrooted |
| `branch_label` | Which set of branch labels are to be displayed | "aa", "lineage" |
| `panels` | List of panels which (if available) are to be displayed | ["tree", "map"] |
| `transmission_lines`| Should transmission lines (if available) be rendered on the map? | Boolean |
Copy link
Contributor

Choose a reason for hiding this comment

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

Might be good (later on) to link to a doc (e.g. this one - although that doc itself looks like a stub/TODO and not included in the auspice docs sidebar) so that folks can find out more about what transmission lines are when reading this doc.

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 Those tutorials (how-to's, really) are desperately needed, but as you note that one's a stub which isn't actually part of the docs.

@jameshadfield jameshadfield merged commit 55208f6 into master Jun 3, 2020
@jameshadfield jameshadfield deleted the transmission-perf branch June 3, 2020 22:16
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