stevenbenner / jquery-powertip

:speech_balloon: A jQuery plugin that creates hover tooltips.
https://stevenbenner.github.io/jquery-powertip/
MIT License
821 stars 137 forks source link

Performance issues while migrating from 1.2.0 to latest version (1.3.1) #178

Closed doxygen closed 2 years ago

doxygen commented 5 years ago

Thanks for the powertip plugin. I use it in my doxygen documentation generator tool, which can be found here https://github.com/doxygen/doxygen. It uses the plugin to provide tooltips for identifiers in source code. For a large file this can easily produce between 1000 and 10000 tooltips.

Recently, I've upgraded from the ancient version 1.2.0 of powertip to the lastest version. All looked fine initially, but I got a user report about dramatically increased render times of the pages. See https://github.com/doxygen/doxygen/issues/6934

I digged into the root-cause and it looks like it is caused by the upgrade to the newer powertip plugin. I've created an isolated test case to reproduce the issue, which can be found in the zip.

powertip-perf-test.zip

The zip file contains 3 versions of a test page with 1000 tooltips (and the script to generate the test page). One test page is using powertip 1.2.0, one is using powertip 1.3.1, and the last one has the following patch applied (which will probably break something, but my test case still works fine):

--- jquery.powertip-1.3.1.js    2019-04-22 19:24:00.000000000 +0200
+++ jquery.powertip-1.3.1.patched.js    2019-04-27 17:17:31.000000000 +0200
@@ -181,7 +181,9 @@
        }

        // remember elements that the plugin is attached to
-       session.elements = session.elements ? session.elements.add(targetElements) : targetElements;
+// COMMENTED OUT
+//                session.elements = session.elements ? session.elements.add(targetElements) : targetElements;
+// END COMMENTED OUT

        return targetElements;
    };

There is a huge performance difference between 1.2.0 and 1.3.1 with patch on the one hand (they render fast), and the standard 1.3.1 version on the other hand (renders very slow).

I hope someone has time to look into this and provide a better solution to improve the performance.

stevenbenner commented 2 years ago

Indeed, looking at the jQuery code, it does make sense that this operation will be slow with very large collections of DOM elements. Since jQuery is maintaining a collection of elements, and each new element collection added to the list will trigger a merge and sort - those operations triggering many iterations.

I need to keep track of the elements that PowerTip has been bound to in order to enable the destroy() behavior. So a performant workaround will need to be found.

It would probably be possible to just use an array of jQuery objects, and not ask jQuery to maintain a real jQuery collection. Though the code won't be as elegant, and destroy() will probably be slower.

I'll look into this. If anyone has any ideas about how to better implement this, I'd love to hear suggestions.

stevenbenner commented 2 years ago

This should be fixed with commit 7f6b90add096235e16c60b1e77bcc635bb95b207. The fix will be included with the next release.

It turns out that it was both the $.add() in the main powerTip() function, as well as the $.not() in the PowerTip .destroy() method. See the explanation in the commit description for more details.

Also, thank you for putting together that test suite. It was very helpful for debugging the problem and verifying the fix!