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 degradation #120

Closed
nidi3 opened this issue Mar 25, 2018 · 10 comments
Closed

Performance degradation #120

nidi3 opened this issue Mar 25, 2018 · 10 comments
Milestone

Comments

@nidi3
Copy link

nidi3 commented Mar 25, 2018

Not really a bug, but performance got measurable worse with versions 1.5.0 and 1.8.1:

test viz 1.0.0 viz 1.5.0 viz 1.8.1
simple 3.7 22.9 51.7
attributes 4.8 26.3 54.2
polygons 1.9 14.7 39.4
records 1.7 22.8 40.2
big 47.9 216.6 206
loc 3.4 20 45.3
cluster 5.9 31.2 53.3

Tests are run on a V8 engine using J2V8.
Numbers are milliseconds averaged over many runs.

Do you have any idea what could be the cause of this?

@magjac
Copy link

magjac commented Mar 25, 2018

@nidi3 I'm interested to see if d3-graphviz add a measurable performance penalty to viz.js. Would it be possible for you to share your benchmark code?

@nidi3
Copy link
Author

nidi3 commented Mar 25, 2018

öm, d3-graphviz has nothing to do with it.
Here's the link to my performance test: https://github.com/nidi3/graphviz-java/blob/master/src/test/java/guru/nidi/graphviz/PerformanceTest.java

@magjac
Copy link

magjac commented Mar 25, 2018

@nidi3 I know. I just wanted to use the same benchmarks. Thanks.

@nidi3
Copy link
Author

nidi3 commented Mar 25, 2018

Ah, I see. Would be interesting to see if you can confirm/reject my measurements.

@mdaines
Copy link
Owner

mdaines commented Mar 26, 2018

Since v1.5.0, a new Emscripten module instance is created for every call to Viz(). I assume this would be the cause of worse average performance over several runs. One reason for doing this was that certain errors could leave the module in a state where it wouldn’t be able to render further graphs. Perhaps it’s time to revisit the fix.

@nidi3
Copy link
Author

nidi3 commented Mar 26, 2018

Thanks for the answer. As I said, it's not a problem for me, but any speed improvement will surely be appreciated.
Interestingly, it seams that J2V8 and graphviz-java are also getting slower in newer versions, I guess for the same reasons (trading speed for correctness/stability).

@mdaines mdaines added this to the v1.9 milestone Mar 27, 2018
@thari
Copy link

thari commented Mar 30, 2018

We should also include the wasm builds in the performance comparisons.

@nidi3
Copy link
Author

nidi3 commented May 15, 2018

Version 2.0.0 brought a HUGE improvement:

test viz 1.0.0 viz 1.5.0 viz 1.8.1 viz 2.0.0
simple 3.7 22.9 51.7 2.2
attributes 4.8 26.3 54.2 2.6
polygons 1.9 14.7 39.4 1.4
records 1.7 22.8 40.2 1.4
big 47.9 216.6 206 19.8
loc 3.4 20 45.3 2.3
cluster 5.9 31.2 53.3 3.2

really great work!

@mdaines
Copy link
Owner

mdaines commented May 15, 2018

Great! The improvement in the average run time is likely due to reusing the Emscripten module instance between calls. In 2.0 you can create a new instance if necessary, so I decided it was OK to reuse it. See the Caveats wiki page for an example.

@mdaines mdaines closed this as completed May 15, 2018
@nidi3
Copy link
Author

nidi3 commented May 15, 2018

Saw it and using it like you suggest 👍

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

No branches or pull requests

4 participants