Skip to content
This repository has been archived by the owner on Jul 29, 2019. It is now read-only.

Improve the performance of the network layout engine #2729

Merged
merged 2 commits into from
Feb 22, 2017

Conversation

richvdh
Copy link
Contributor

@richvdh richvdh commented Feb 15, 2017

The performance of the network layout engine is quite poor for highly-connected networks. In the worst case of the fully-connected graph, a number of the methods are O(2^n) in the number of nodes, which is disastrous for large networks.

This PR tweaks the worst culprits to maintain a "visited" map which allows them to only visit each node once, hence limiting them to O(n).

Short-cut the execution of a number of methods in LayoutEngine to make them
handle highly-connected graphs better.
@@ -532,8 +543,8 @@ class LayoutEngine {
let diffAbs = Math.abs(pos2 - pos1);
//console.log("NOW CHEcKING:", node1.id, node2.id, diffAbs);
if (diffAbs > this.options.hierarchical.nodeSpacing) {
let branchNodes1 = {}; branchNodes1[node1.id] = true;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

getBranchNodes now requires an empty map rather than one in which the first node is populated; since getBranchNodes always populates the map for the first node, this was redundant anyway.

@Tooa
Copy link
Member

Tooa commented Feb 16, 2017

Can we test this somehow? Can you provide us an example showing your improvements for a very dense network?

@Tooa Tooa added the Network label Feb 16, 2017
@Tooa Tooa added this to the Minor Release v4.19 milestone Feb 16, 2017
@richvdh
Copy link
Contributor Author

richvdh commented Feb 16, 2017

Have a look at the examples at richvdh@d21fe3b. demo.jsonp takes 250ms to lay out here, versus 20 seconds with vis 4.18.1. I've not had the patience to wait for 4.18.1 to complete laying out demo_big.jsonp.

@Tooa
Copy link
Member

Tooa commented Feb 16, 2017

Nice! Thank you very much. It looks good to me. @mojoaxel can you have a look at this too? I want to make sure to not overlooking something.

@mojoaxel mojoaxel self-requested a review February 16, 2017 14:11
@mojoaxel
Copy link
Member

@richvdh Thanks very much!
Please give us some time to review your changes and test them against our existing examples. This is really amazing but we want we make sure not to introduce new bugs (we already have enough 😉 )

@mojoaxel
Copy link
Member

Have a look at the examples at richvdh/vis@d21fe3b

@richvdh Can you add these example to the pull request? It's always good to have new examples/tests for "new" features.

@richvdh
Copy link
Contributor Author

richvdh commented Feb 16, 2017

Can you add these example to the pull request? It's always good to have new examples/tests for "new" features.

I'm happy to just add them as-is if you're happy with that - but they're far from polished, and I'm reluctant to set off down a path of spending more time on that.

@mojoaxel
Copy link
Member

I'm happy to just add them as-is if you're happy with that

Yeah that's fine. Just make sure the example is working.

@richvdh
Copy link
Contributor Author

richvdh commented Feb 16, 2017

Ok, done.

(don't think the travis failure is my fault!)

@mojoaxel
Copy link
Member

OK, now we have kind of a problem.
@ludost also provides with #2759 a big change on the LayoutEngine.
Whatever pull-request gets merged first makes merging for the other one really hard.

If I had to decide, I would say we write some nice performance tests and run both pull-requests against develop. For me this is the only way to decide which pull-request to merge first.

Of course @ludost is the official maintainer and has the last word on this. In my opinion it would be the best for @ludost to talk directly to @richvdh and figure out how to deal with this performance thing!

Well, multiple competing pull-requests is a good thing after all!

Copy link
Member

@mojoaxel mojoaxel left a comment

Choose a reason for hiding this comment

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

On hold until it is clear how to deal with #2759

@ludost
Copy link
Member

ludost commented Feb 22, 2017

Hi,

I suggest merging this one first and let me handle the conflict, as my pull request is still too rough.

Greetings,
Ludo.

@mojoaxel mojoaxel merged commit 6acc7bd into almende:develop Feb 22, 2017
marcortw pushed a commit to marcortw/vis that referenced this pull request Mar 12, 2017
…lmende#2729)

* Improve the performance of the network layout engine

Short-cut the execution of a number of methods in LayoutEngine to make them
handle highly-connected graphs better.

* Demonstrations of layouts of large networks
yotamberk pushed a commit that referenced this pull request May 2, 2017
* Add Gitter badge (#2179)

* do not generate source-maps in distribution version

* add 'dist' folder for deployment

* added Badges

* added codeclimate badge

* added @Tooa to the support team

* added badges from isitmaintained.com (#2517)

* do not ignore dist and test folders in master

* generated dist files for v4.18.0

* generated dist files for v4.18.1

* Cheap fix for bug #2795

* Update to PR #2826 to use newline format

* changed to v4.18.1-SNAPSHOT

* chore(docs): general improvements (#2652)

* removed NOTICE file

* updated license date range to include 2017

* chore(docs): updated support team members

* chore: updated dependencies and devDependencies (#2649)

* Fixes instanceof Object statements for objects from other windows and iFrames. (#2631)

* Replaces instanceof Object checks with typeof to prevent cross tab issues.

* Adds missing space.

* chore: removed google-analytics from all examples (#2670)

* chore(docs): Add note that PRs should be submitted against the `develop` branch (#2623)

Related to: #2618
Related to: #2620

* feat(timeline): Change setCustomTimeTitle title parameter to be a string or a function (#2611)

* change setCustomTimeTitle title parameter, Now could be an string or a function
* Fixed indent and spacing

* feat(timeline): refactor tooltip to only use one dom-element (#2662)

* feat(network): Allow for image nodes to have a selected or broken image (#2601)

* feat(tests): run mocha tests in travis ci (#2687)

* Added showX(YZ)Axis options to Graph3d (#2686)

* Added showX(YZ)Axis to Graph3d

* Added show_Axis options to docs and playground example

* Resolved merge conflict

* Added show_Axis options to docs and playground example

* fix(build): use babel version compatible with webpack@1.14 (#2693)

fixes #2685

* feat(docs): use babel preset2015 for custom builds (#2678)

* add link to a mentioned example (#2709)

* chore(lint): added support for eslint (#2695)

* Trivial typo fix in how_to_help doc. (#2714)

* fix(timeline): #2598 Flickering onUpdateTimeTooltip (#2702)

* Fix redraw order
* Fix error when option is not defined
* Allow template labels
* Add .travis.yml file
* Add experiment travis code
* Fix react example
* Add animation to onUpdateTooltip

* fix(timeline): #778 Tooltip does not work with background items in timeline (#2703)

* Fix redraw order
* Fix error when option is not defined
* Allow template labels
* Add .travis.yml file
* Add experiment travis code
* Fix react example
* Make items z-index default to 1

* Add initial tests for Timeline PointItem (#2716)

* fix(timeline): #2679 TypeError: Cannot read property 'hasOwnProperty' of null (#2735)

* Fix redraw order
* Fix error when option is not defined
* Allow template labels
* Add .travis.yml file
* Add experiment travis code
* Fix react example
* Fix bug in item editable

* feat(timeline): #2647 Dynamic rolling mode option (#2705)

* Fix redraw order
* Fix error when option is not defined
* Allow template labels
* Add .travis.yml file
* Add experiment travis code
* Fix react example
* Add toggleRollingMode option
* Update docs with toggleRollingMode option

* fixes timestep next issue (#2732)

* feat(timeline): added new locales for french and espanol (#2723)

* Fix eslint problem on Travis. (#2744)

* fix: Range.js "event" is undeclared (#2749)

* fix(timeline): #2720 Problems with option editable (#2743)

Clean up and centralise edit status for Timeline Items.

* feat(network): Improve the performance of the network layout engine (#2729)

* Improve the performance of the network layout engine

Short-cut the execution of a number of methods in LayoutEngine to make them
handle highly-connected graphs better.

* Demonstrations of layouts of large networks

* Added support to supply an end to bar charts to have them scale (#2760)

* Added support to supply an X2 to bar charts to have them scale

* Fixed graph2d stacking issue.  It no longer takes into account hidden items

* Changed x2 to end per recommendation and added this to the docs

* Initial tests for timeline ItemSet. (#2750)

Somewhat more complicated setup, associated with the need for a real window.

* [Timeline] Modify redraw logic to treat scroll as needing restack. (#2774)

This addresses #1982 and #1417.

It possibly reduces performance, but correctness seems better.

* fix(timeline): #2672 Item events original event (#2704)

* Fix redraw order

* Fix error when option is not defined

* Allow template labels

* Add .travis.yml file

* Add experiment travis code

* Fix react example

* Fix events returned from mouse events

* Fix example

* Rename censor to stringifyObject in example

* [timeline] Update examples to use ISOString format. (#2791)

Resolves #2790

* [timeline] Update serialization example to use ISOString dates. (#2789)

Resolves #2696

* added github templates for issues and pull-requests (#2787)

fixes #2418

* feat(timeline): Add item data as argument to the template function (#2799)

* Fix regression introduced in #2743. (#2796)

* Fix for issue #2536 (#2803)

* Fix for issue #2536

* Adjusted documentation for fix.

* Adjustments due to review

* Grrrrr whitespace

* Fixed Travis issue

* Cheap fix for bug #2795

* Update to PR #2826 to use newline format

* Update to gitignore to reflect changes on remote

* clean dist folder

* Local gitignore update

* Just a first example file for the week scale feature

* Allowing sourcemap creation

* Initial (non-functional) commit to ensure we insert code at the right places (check TODOs)

* Functional, not bug-free version which works with locale aware week numbers.

* Locale-aware implementation and simplified major labels to a full year

* Trying to make the major labels show the correct start date

* Working implementation of week numbers using localization.

* removing development leftovers

* Updated package.json

* Reintagrate package.json from accidental deletion

* Updates for package.json

* Fixing package.json

* Integrate the week numbers feature in the documentation.

* Reverting local changes to .gitignore

* Reverting local changes

* Extending examples to cover the case when 1st day of week and 1st day of month align; Fixing display bug so that week numbers are not repeated in minorLabels

* Putting the examples into a loop
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants