ugurdogrusoz / visuall

Visuall: A tool for convenient construction of a web based visual analysis component
2 stars 0 forks source link

Label truncation performance improvement #219

Closed ugurdogrusoz closed 3 years ago

ugurdogrusoz commented 4 years ago

In order to improve the performance due to label truncation (when Fit labels to nodes is true), which seems to be an expensive operation, we could truncate them just once at the beginning of the element instantiation, save it in a new data field, and then use that field all the time.

ugurdogrusoz commented 4 years ago

Let's revisit this issue in line with what we discussed please. Briefly the goal here is to set the text-wrap style different than user specifies with the application for performance reasons as follows (how it is in the application per map -> how it should be in Cytoscape.js per node):

canbax commented 4 years ago

If we use a simple function to truncate the node labels and let text-wrapping be empty, I see a small performance improvement If we use a complex function (from the source of cytoscape.js) and do the same, I see a huge performance decrease. Currently, we rely on setting text-wrapping: wrap or text-wrapping: ellipsis. I think it is fine because when we provide a simple function for truncation, our function would be not robust for style changes in app_description.json. e.g. if the user sets a very big font-size for some type of node, our simple method doesn't consider font-size or font-family and gives bad results.

ugurdogrusoz commented 4 years ago

Let's try to use a moderate function (as in Newt) to truncate or wrap (in case of ellipses and wrap, resp.) and see if the performance improves.

ugurdogrusoz commented 4 years ago

@hasanbalci Please review and advise suitability for Newt.

hasanbalci commented 4 years ago

For queries, I couldn't see an improvement for "wrap" and "ellipses" options, only new "none" option provides a better performance. During layout, I think text-wrap type doesn't have an effect on simple graphs. It may have an effect on compound graphs but I am not certain on it.

ugurdogrusoz commented 4 years ago

Looks like we ve done something useless and also complicated things. I suppose we should roll back and stick to core Cytoscape.js functionality. It might be nice to keep the related option in setting as radio buttons of three available choices though. Let's talk.

canbax commented 4 years ago

I did some performance tests to see the difference. I see that the pre-calculation of labels performs better than setting text-wrap: ellipsis when I load data first.

When I do layout after the first load I didn't see much difference.

In addition, I tried to add classes to nodes by checking if the label needs to be truncated. For example if a node label is already short does not need to be truncated, don't touch it. This requires checking each node in for loop and adding class if necessary. Since there is for loop, this gave worse results. Since the results are a lot worse, I didn't make tests with this case.

The details of tests are below https://docs.google.com/document/d/1jSw1fHaqxgTWX1ArUzQG9pHnM-MCIdUiU47u8qdEALk/edit?usp=sharing

ugurdogrusoz commented 4 years ago

Results look good. Let's just do a few similar tests on a different machine (Macbook?) as well and then decide.

canbax commented 4 years ago

I made tests with Macbook. Results are similar to previous ones. On the first loads, manual labeling performs better.

ugurdogrusoz commented 3 years ago

Let's close this at least until somebody brings this up as a performance issue.