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

Issue performance clearTimeout (especially in use-anchor.js) #15203

Closed
felipesdias opened this issue Dec 31, 2022 · 5 comments
Closed

Issue performance clearTimeout (especially in use-anchor.js) #15203

felipesdias opened this issue Dec 31, 2022 · 5 comments

Comments

@felipesdias
Copy link

What happened?

When there are many tooltip components, destroying the component can compromise performance.
I ran some profiles and found that the problem is with the clearTimeout calls in the use-anchor and use-timeout.

In my case, when switching pages in my application, it was taking 500ms-1000ms just to destroy the components (after the workaround below, the time was reduced to 40ms-80ms).

I did a performance test comparing the clearTimeout call with undefined and only calling it when the ID exists .

For now, I have replaced the default clearInterval and clearTimeout methods.

    const originalClearInterval = clearInterval;
    const originalClearTimeout = clearTimeout;

    clearInterval = id => {
        if (id) originalClearInterval(id);
    };
    clearTimeout = id => {
        if (id) originalClearTimeout(id);
    };

What did you expect to happen?

Before executing clearTimeout and clearInterval in all calls, should check if the ID is defined, for example as follows:

// use-anchor.js
  onBeforeUnmount(() => {
    clearTimeout(touchTimer)
    unconfigureAnchorEl()
  })

to:

// use-anchor.js
  onBeforeUnmount(() => {
    if (touchTimer) {
        clearTimeout(touchTimer)
    }
    unconfigureAnchorEl()
  })

Reproduction URL

https://jsbench.me/i4lcbtfdx4/1

How to reproduce?

Create a many components with tooltip and destroy then

Flavour

Quasar CLI with Webpack (@quasar/cli | @quasar/app-webpack)

Areas

Composables (quasar)

Platforms/Browsers

Chrome

Quasar info output

Operating System - Windows_NT(10.0.22000) - win32/x64
NodeJs - 16.14.2

Global packages
  NPM - 8.9.0
  yarn - 1.22.15
  @quasar/cli - 1.3.2
  @quasar/icongenie - Not installed
  cordova - Not installed

Important local packages
  quasar - 2.11.3 -- Build high-performance VueJS user interfaces (SPA, PWA, SSR, Mobile and Desktop) in record time
  @quasar/app-webpack - 3.6.2 -- Quasar Framework App CLI with Webpack
  @quasar/extras - 1.15.9 -- Quasar Framework fonts, icons and animations
  eslint-plugin-quasar - Not installed
  vue - 3.2.41 -- The progressive JavaScript framework for building modern web UI.
  vue-router - 4.1.6
  pinia - Not installed
  vuex - 4.1.0 -- state management for Vue.js
  electron - Not installed
  electron-packager - Not installed
  electron-builder - Not installed
  @babel/core - 7.19.6 -- Babel compiler core.
  webpack - 5.74.0 -- Packs CommonJs/AMD modules for the browser. Allows to split your codebase into multiple bundles, which can be loaded on demand. Support loaders to preprocess files, i.e. json, jsx, es7, css, less, ... and your custom stuff.
  webpack-dev-server - 4.9.3 -- Serves a webpack app. Updates the browser on changes.
  workbox-webpack-plugin - Not installed
  register-service-worker - 1.7.2 -- Script for registering service worker, with hooks
  typescript - 4.5.5 -- TypeScript is a language for application scale JavaScript development
  @capacitor/core - Not installed
  @capacitor/cli - Not installed
  @capacitor/android - Not installed
  @capacitor/ios - Not installed

Quasar App Extensions
  @quasar/quasar-app-extension-qmarkdown - 2.0.0-beta.7 -- Display inline markdown in your Quasar App

Networking
  Host - DESKTOP-ENGH6PL
  Wi-Fi - 192.168.2.100
  vEthernet (WSL) - 172.18.192.1

Relevant log output

No response

Additional context

No response

@github-actions
Copy link

Hi @felipesdias! 👋

It looks like you provided an invalid or unsupported reproduction URL.
Do not use any service other than Codepen, jsFiddle, StackBlitz, Codesandbox, and GitHub.
Make sure the URL you provided is correct and reachable. You can test it by visiting it in a private tab, another device, etc.
Please edit your original post above and provide a valid reproduction URL as explained.

Without a proper reproduction, your issue will have to get closed.

Thank you for your collaboration. 👏

@rstoenescu rstoenescu self-assigned this Jan 3, 2023
@rstoenescu
Copy link
Member

Hi.

Your benchmark is not correct, since the second method does not executes clearTimeout at all. So this is not a fair comparison.

@rstoenescu
Copy link
Member

Will investigate though.

@yusufkandemir
Copy link
Member

@rstoenescu that's the main point, avoiding the slow clearTimeout call by using a check beforehand.

Aside from that, to complement the benchmark, one should create actual timeouts, and check how the additional check affects the scenarios where there is an actual timeout to clear.

@rstoenescu
Copy link
Member

@felipesdias

Thank you for investigating and the detailed explanation.
Perf improvements (not just for use-anchor, but globally throughout Quasar UI) will be available in Quasar v2.11.4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants