Skip to content

Do not sanitize tooltip if it does not contain any html

Himanshu Kapoor requested to merge himkp-perf-tooltip into master

Background

In Bootstrap Tooltip's default config settings we have the below default settings (check for yourself by typing Tooltip.Default in the console):

{
  "template": "<div class=\"tooltip\" role=\"tooltip\"><div class=\"arrow\"></div><div class=\"tooltip-inner\"></div></div>",
  "sanitize": true,
}

There are many more keys here but the two above are the most significant for this change. For most (more than 90% of the tooltips), these two default settings never change. However, despite that, Bootstrap decides it is important to sanitize the default template each time a tooltip object is created:

Screenshot_2020-09-28_at_7.34.41_PM

The sanitize function takes about 3-5ms to execute, which normally isn't a big deal. However if you take 3-5ms of processing and add a tooltip to say thousands of elements in DOM, you easily end up with a slowdown.

The issue in the Web IDE

In the case of the Web IDE, consider the left sidebar. Each item in the left sidebar has a dropdown menu that contains around 5 submenu items, each of which have a v-tooltip directive on them.
Screenshot_2020-09-28_at_7.26.45_PM

Considering about 5ms of time to render a tooltip which is sanitized, you get about 25-30ms of execution time to render a list item. And in the gitlab-org/gitlab, the root directory has 65 files, which means 1625-1950ms of execution on initial load, theoretically.

The fix

By simply telling Bootstrap to not sanitize the default template since it never changes in the tooltip directive, we bring down the execution of tooltip initialisation from 3-5ms to 0.3-0.5ms (around 90%). The only exception is if the data-html=true attribute is passed, in which case we do need to sanitize our input. (However, that rarely happens in our codebase, and not at all in the Web IDE module.)

Before the fix

In the left sidebar, consider expanding a folder that has 11 items.
Screenshot_2020-09-28_at_7.52.12_PM

Expanding it mounts 11 items, which take about 24-25ms each.
Screenshot_2020-09-28_at_7.32.08_PM

Each item contains 5 menu items that contain 1 tooltip each, which take 3-5ms each.
Screenshot_2020-09-28_at_7.32.48_PM

Finally, it boils down to the culprit function sanitizeHtml which takes up 3-5ms:
Screenshot_2020-09-28_at_7.33.10_PM

After the fix

Each item now takes 6-8ms to render, and the overall render time for 11 elements is cut down from 280ms to 80ms. (Roughly 70% performance gain.)
Screenshot_2020-09-28_at_7.56.00_PM

Impact

Based on the observations above, we should see a reduction in page load time for the Web IDE by around 1200ms for the gitlab-org/gitlab project, and a 70-90% improvement in performance for expanding folders in the left sidebar.

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by Himanshu Kapoor

Merge request reports