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

vis.js is getting really slow and laggy when using circularImages #2952

Closed
andrasbarabas opened this issue Apr 5, 2017 · 26 comments
Closed

Comments

@andrasbarabas
Copy link

andrasbarabas commented Apr 5, 2017

Hello!

I'm not sure if this problem has been answered earlier, to be honest I haven't found anything related so far.
My issue is, I've got a graph with something like 40 nodes and 30 edges. Basically I'm working on a family tree, and these nodes contain a mini-profile image fetched from Facebook, as circularImage shape. Of course, these are just thumbnails, JPGs and usually around 5-10 kbyte. Still when all the 40 nodes load in, my vis network gets really laggy and whenever I move a node around (I even turned off physics!), Chrome eats up like 1-1,5 GB RAM instantly. Is there any fix for this issue or do I miss something maybe? Is there a physics setting for the nodes as well, next to the network?

Thank you in advance!

@wimrijnders
Copy link
Contributor

wimrijnders commented Apr 10, 2017

Hi there, I would like to look into this issue. Is it possible that you post an example network to test the issue? Something without the thumbnails actually used is fine. Just plunk in an example image.

Chrome is a notorious memory hog, but I'd like to investigate why for the case of dragging nodes around memory usage increases so drastically. In any case, dragging around any old network does not seem to have much impact on the memory.

TIA.

@andrasbarabas
Copy link
Author

Yes, there is an example, actually the current project I'm working on. Although I'm not sure why you do want to check out a vis network without the thumbnails (as I mentioned, I'm experiencing this lag when the thumbnails are on & the shapes are circularImages, if I use simple circles without images, everything is fine).

Well yeah, if I use images on the nodes, and drag a node (even if I have like 10 nodes with 8 edges or something like that) I immediately go up to 1500 MB RAM usage from something like 300.

Here is the site: http://ifatest.bceocsi.com/
Sorry about the language though, try to avoid it, the vis network is already there if you just simply load in the site. :)

@wimrijnders
Copy link
Contributor

Although I'm not sure why you do want to check out a vis network without the thumbnails

This is actually more of a privacy thing; I understand that you are displaying family members and I didn't really want to use their pictures to test; you never know where they might end up. I was thinking of adding own thumbnails.

@wimrijnders
Copy link
Contributor

wimrijnders commented Apr 12, 2017

I managed to replicate the network as on the given site:

untitled

Can you now show me how you added the images to the nodes? Just an example of one node will do. So, for example, what do you add to the following for an image?

		{
			id:203,
			label:"Ábrányi Vanda",
			size:50,
			color:"#6aa84f",
			font:{color:"#000000"},
		},

Thanks.

@andrasbarabas
Copy link
Author

andrasbarabas commented Apr 12, 2017

Of course I can show you. :) And you don't have to worry about it, because at the moment, minimal amount of pictures are uploaded, I'm using a facebook dummy image on most of the nodes (a thumbnail one), yet it's lagging like hell.

http://ifatest.bceocsi.com/public/js/tree.js
This is the file which is generating the graph. The image adding part is at the very first else statement:

instructorData.push({
  shape: 'circularImage',
  image: 'public/resources/thumbnails/' + instructors[i].picture,
  id: instructors[i].id,
  label: instructors[i].full_name,
  size: 50,
  color: color,
  font:
  {
    color: font_color
  }
 });

@wimrijnders
Copy link
Contributor

Thanks for the quick response. I'll see if I can work on it further later today.

@andrasbarabas
Copy link
Author

Thank you very much, I really appreciate your effort and time! :)

@wimrijnders
Copy link
Contributor

I haven't had time to look at it in depth, but I have verified that the memory usage goes crazy: over 30% on my machine (16GB). And that is with using the same one image for all nodes.

I'm doing some investigation in the code; at least, I hope it is something in there. It could also be a problem with chrome itself.

@andrasbarabas
Copy link
Author

I tested it too yesterday and experienced the same. With the same image on like 40 nodes, it was laggy. After that I used a PNG instead of the jpg one, and noticed something. It's not only about the file size, even if the file was around 5kb it was laggy as hell. The new image's dimension was 50px50px (so it was fixed sized thumbnail, on the backend side I used a % based calculation before which is not an optimal solution in every case). With the 5050 png it was fine though. At least I didn't experience skyrocketing memory usage, it stood around the same value when I dragged the graph.

Oh and Firefox has the same issue, so I guess it's not related to some browser glitch.

@wimrijnders
Copy link
Contributor

wimrijnders commented Apr 13, 2017

So with the small image, the memory abuse is not so bad, but there is still lag. I don't experience any lag at all, but this is probably because I've got a pretty heavy development machine.

Oh and Firefox has the same issue, so I guess it's not related to some browser glitch.

This is good to know; it means that I won't be searching in vain.

@wimrijnders
Copy link
Contributor

Duplicate of #2408. I will assume this issue to be leading.

@wimrijnders
Copy link
Contributor

wimrijnders commented Apr 21, 2017

OK, managed to do a profiling run for this issue. The test is:

  • Start the CPU profiling (Chrome)
  • drag nodes around for some time
  • Stop the profiling

The culprit is most definitely CircleImageBase._drawImageAtPosition(), with a single loop there taking up 76% of the execution time. This explains the lagging to a large extent.

Screenshot of profiling, note the value 256.4 ms, where the main issue lies:

untitled

In addition, canvas elements are created and discarded here like there's no tomorrow. I'm not profiling the heap, but I image that the garbage collector is screaming for mercy when dragging is performed.

Also the temporary canvas is rewritten multiple times (if you're unlucky), with only the final result being displayed.


So, this code needs some serious attention. The most obvious things to look at are:

  • The multiple drawImage() calls in the loop; these shouldn't be necessary
  • Examine if images can be preprocessed to Canvas. Googling tells me that there might be issues with scaling

Other suggestions are welcome.

@andrasbarabas
Copy link
Author

So, if I'm correct, the script tries to draw the same canvas (and all of its nodes) several times if you move a node around (or the whole graph if it is connected)?

@wimrijnders
Copy link
Contributor

wimrijnders commented Apr 21, 2017

@eyho yes, more or less. This actually only happens if the network is zoomed out a lot, and the nodes/images are small (ref factor > 2 test).

...More specifically, a temporary canvas is created for every image, written to (perhaps multiple times) and copied to the main canvas. This happens on every redraw.

What makes it worse, is that this happens for all the images in all the nodes, even if said image has been cached and used multiple times. Major performance hit; I think this can be avoided.

@rasmusblockzero
Copy link

Without having looked at the example at all I just want to throw in the idea that one issue might be
updateShape()

The nodes handler version of setOptions
triggers in my app on zoom I think, without me resetting the shape value
this.body.nodes[nodeId].updateShape();

get's called on every node which result in a new shape created for every node.
If the image is somehow allocated for the shape this might lead to a lot of memory before the garbage collector finds it

There is no delete there just an overwrite
this.shape = new XXX

Just a thought...

(One very straightforward fix if this turned out to be the problem would be to let the node know the latest shape and not recreate if it's the same)

@wimrijnders
Copy link
Contributor

@rasmusblockzero Yes, updateShape() has issues of its own and is also in need of refurbishment. However, it is not in the scope of this issue. updateShape() is called when setting options, and this issue deals with performance in the drawing loop.

(One very straightforward fix if this turned out to be the problem would be to let the node know the latest shape and not recreate if it's the same)

It is not the problem here; updateShape() is called when options are changed, not during drawing. It has a specific function, namely to change the shape of a node at runtime. Also, the shape is remembered, it is stored in member variable shape of a Node instance.

This is stuff for a separate issue or PR. Feel free to work on this.

@wimrijnders
Copy link
Contributor

Upon investigating, I can understand the rationale behind the code in CircleImageBase._drawImageAtPosition(). It is a technique to reduce interpolation artifacts in images when scaling. I believe the linked-to page was the very inspiration for this code.

So there is a good reason that this code exists; Alex de Mulder, you did a great job.

However, doing this in realtime on every drawing iteration is enormous overhead. My thought is to cache the interim results of the interpolation per image. Something like mipmapping. Feel free to share your thoughts on this.

@rasmusblockzero
Copy link

@wimrijnders In my case he updateShape is called far too much (I only use one type of shape), it appears to happen on redraw after zoom. What is saved in the node is the pointer to the shape, not the string of the shape type, correct? so when updateShape() is called without argument the object is always recreated even if the type is the same.
I believe you if you have concluded that it doesn't have anything to do with this though :-) it just came to mind as I saw this investigating something else the other day - and having discussed it here I have even more incentive to investigate. I'll get back on that :-)

@rasmusblockzero
Copy link

@wimrijnders caching the result make a lot of sense and should be pretty limited change too.

(If a fix is needed in the meantime shapeProperties.interpolation can perhaps be turned from a drag event call back)

@wimrijnders
Copy link
Contributor

@rasmusblockzero Yes, updateShape() has problems I'm aware of. For this current issue, these problems are not relevant, because updateShape() is not even being called.

(If a fix is needed in the meantime shapeProperties.interpolation can perhaps be turned from a drag event call back)

Not a good idea; a redraw can be fired for other reasons than a drag, e.g. when physics is turned on.

@wimrijnders
Copy link
Contributor

Upon delving into the code, I realized there is a very simple workaround for the memory issues. In order to avoid the intensive zooming, simply set the following option:

nodes: {
  shapeProperties: {
    interpolation: false    // 'true' for intensive zooming
  }
}

This skips the code causing the performance issues, at the expense of perhaps worse-looking images.

@rasmusblockzero
Copy link

Agree, that's the option I meant, sorry if I was unclear.

My thought was that if interpolation is turned off when drag starts and back on when drag finish that might be something intermediate - but that would obviously not solve the memory issue.

@wimrijnders
Copy link
Contributor

@rasmusblockzero Ah, OK, we were talking about the same thing. Sorry for the misunderstanding.

@andrasbarabas
Copy link
Author

Damn, interpolation setting has really had an effect on my network. Thank you!! (Although it won't solve the image problem, it really came in handy :))

@wimrijnders
Copy link
Contributor

@eyho If you're satisfied with the answer, please close the issue. There is a workaround now, and a more general fix is pending.

@wimrijnders
Copy link
Contributor

Thanks!

yotamberk pushed a commit that referenced this issue May 13, 2017
* CachedImage with preredendered zoom images; first working version.

* Fixes for missing images and usage brokenImage.

* Remove unused method

* Added height member of CachedImage, note about svg's
@mojoaxel mojoaxel added this to the Minor Release v4.20 milestone May 19, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants