matomo-org / matomo

Empowering People Ethically with the leading open source alternative to Google Analytics that gives you full control over your data. Matomo lets you easily collect data from websites & apps and visualise this data and extract insights. Privacy is built-in. Liberating Web Analytics. Star us on Github? +1. And we love Pull Requests!
https://matomo.org/
GNU General Public License v3.0
19.93k stars 2.66k forks source link

Beforeunloadhandler blocks UI #21297

Open mgrubinger opened 1 year ago

mgrubinger commented 1 year ago

Context

I have investigated the cause for slow INP on some of our pages and discovered that links which also send a matomo event via onclick=_paq.push(['trackEvent' ... (simplified) delay the navigation to the next page for at least 100ms.

This significantly slows users perceived performance of the site and negatively impacts INP score.

Screenshot of beforeonload event in chrome performance measurement: 100msdelay

Related issues:

The cause for this delay seems to be https://github.com/matomo-org/matomo/blob/5.x-dev/js/piwik.js#L522 where the main thread is deliberately blocked by a do/while loop.

Expected Behavior

The navigation event should not be blocked.

Current Behavior

All links that also have an onclick event handler sending a matomo event are delayed by 100ms due to the do/while loop in https://github.com/matomo-org/matomo/blob/5.x-dev/js/piwik.js#L522 (or L520 in 4.x-dev)

The do/while loop executes ~5500 times on my macbook pro 15 (intel), with simulated 4x CPU slowdown ~1500 times.

Possible Solution

In my experiments, the beacon/ping request was sent even when removing the do/while loop. From what I gathered in the linked issues (above) the forced delay by occupying the main thread with a loop should not be required any more when sendBeacon is used?

Screenshot of beforeunload event after removing the do/while loop withoutvondelay

Steps to Reproduce (for Bugs)

  1. create a link with an onclick event handler (either as attribute or addEventListener('click') that sends a matomo event: _paq.push(['trackEvent', 'sidebarnavigation', 'click', 'Settings'); or similar on a website with matomo tracking acitvated
  2. open chrome dev tools, select performance panel
  3. click record in performace panel
  4. click link
  5. stop record in performance panel
  6. you should see that the beforeunload handler takes ~100ms due to the do/while loop in L522

Your Environment

sgiehl commented 1 year ago

Hi @mgrubinger Thanks for creating this issue. I would agree that doing that page block shouldn't be needed when using sendBeacon. It's actually documented that sendBeacon is meant to track analytical data even on page unload. See https://developer.mozilla.org/en-US/docs/Web/API/Navigator/sendBeacon

@tsteur Do you think it makes sense to avoid removing that wait time when sendBeacon can be used?

tsteur commented 1 year ago
image

Seems we're already waiting only 100ms instead of 500ms in those cases. If it can be fully removed or lowered further I don't know and would need to be tested across browsers in various different scenarios including closing the window, closing tab, navigating to different sites etc

mgrubinger commented 1 year ago

For what it's worth: we patched matomo.js to remove the 100ms (setExpireDateTime) last week and did not see significant changes in the events that are reported in matomo. See attached a screenshot of one of the events in question that is triggered on navigation (anchor link onclick).

Changes were deployed on 10th of October, note that on 10th and 11th of Oct 2023 we saw increased traffic on our site due to Amazon Prime Deals Days.

If it's of any help we can try to look into the data more, focussing on older/specific browsers? Let me know if you need any specific data or more observations.

Screenshot of Matomo GUI showing a graph of events from 2023-10-02 to 2023-10-15

LeoniePhiline commented 1 month ago

We are also experiencing bad INP ratings caused by matomo's delay, using Matomo 5.

This bad INP rating appears to negatively affect page rank.

Is there any progress on this matter?

According to https://developer.mozilla.org/en-US/docs/Web/API/Navigator/sendBeacon#description, no delay should be required any more at all in the unload handler.