Could Sentry javascript performance be improved?

[OZZlE]

Important Details

How are you running Sentry?

• On-Premise docker [Version xyz]
• Saas (sentry.io)
• Other [briefly describe your environment]

Description

When measuring mobile 4G performance using Google Chrome Audit (Lighthouse) removing Sentry javascript seems to improve our performance score around 3% on initial page load.

Chrome also reports (if you switch on all logging) in console:

bundle.min.js:2 [Violation] 'setInterval' handler took 239ms
bundle.min.js:2 [Violation] 'setInterval' handler took 51ms
bundle.min.js:2 [Violation] 'setInterval' handler took 117ms
bundle.min.js:2 [Violation] 'setInterval' handler took 58ms

We are using version 5.2.1

Steps to Reproduce

1. Place Sentry.io javascript (at the very end of body - after React) on a React website with infinite scroll and scroll to load a lot of content
2. Watch Chrome console (with all logging enabled)

[kamilogorek]

@OZZlE is there a chance you have such an app laying around somewhere, and I could use it to profile the issue?

[OZZlE]

@kamilogorek I am not sure if I am allowed to share it publicly, in any case setTimeout/setInterval should be avoided as much as possible - not always possible, but interested as to why Sentry needs to use it?

[OZZlE]

From Google Page speed. I can send you an url privately perhaps.

(p.s. nobody will ever know what 'Other' is...)

[rtymchyk]

Consider code splitting sentry out and loading it at runtime when an error is reported.

[pats]

Guys, it's getting worse every day ... currently it looks like some kind of regression issue

[pats]

I'm guessing but maybe issue was introduced here c3b7940 ?

[OZZlE]

Consider code splitting sentry out and loading it at runtime when an error is reported.

??? can't exactly put try-catch around entire bundle.. even then how would sentry pick it up if it wasn't loaded before? also the try-catch would prevent any error reported unless you throw it again after loading sentry..?

[kamilogorek]

how would sentry pick it up if it wasn't loaded before?

We have lazy-loading solution explained here: https://docs.sentry.io/platforms/javascript/#lazy-loading-sentry

[rtymchyk]

Consider code splitting sentry out and loading it at runtime when an error is reported.

??? can't exactly put try-catch around entire bundle.. even then how would sentry pick it up if it wasn't loaded before? also the try-catch would prevent any error reported unless you throw it again after loading sentry..?

Add global handlers yourself, via window.onerror (and similar handler for uncaught promises). On error, load the module, initialize sentry (disabling their global handler integration), and bubble the exception directly to sentry.

[intellix]

Speaking of code splitting. I feel like the bundle is large at 57.7kb. Does this include the Sentry.showReportDialog({ eventId }) that we don't want/use?

[kamilogorek]

Does this include the Sentry.showReportDialog({ eventId }) that we don't want/use?

No, it lives outside the SDK and is dynamically included when necessary.

[dryoma]

Hi everyone,
I'd like to join in the request as well :)

As for

We have lazy-loading solution explained here: https://docs.sentry.io/platforms/javascript/#lazy-loading-sentry

We are using the lazy loader, and Sentry's bundle is by far the biggest slowdown for us, at least according to Google (especially prominent on mobile)

[sod]

you have to remove the Breadcrumb Integration as it's a) bugged anyway and b) shatters your performance. See #2125

[OZZlE]

Say that your entire website consists of One JS App for example in React. To catch js errors one could wrap the entire app in index.js with a try { } catch(e) { // send to Sentry }

In that case Sentry would only need to provide a tiny js API to send errors to it and you can just use the Sentry "Control panel" part to track errors :)

I have implemented this but still have all the overhead of Sentry js.. also the documentation is not clear about how I send the stack trace or where the error actually originated from.. Maybe what I missed is to after catching and loading up sentry I should throw the error again.

I do not load Sentry.io js at all before there is an actual error this way. Problem is I've lost the stack trace and where the errors are coming from in code. But my suggestion above might fix this.

Seems also like React "warnings" are for some reason caught like this so most of the time Sentry.io js is loaded anyway as it can be pretty hard to always track mounted/unmounted async stuff in React.

[AbhiPrasad]

#3208 will have fixed some performance concerns, and #2707 tracks the bundle size concerns. Additional information can be found here #2817, our plans for a major bump of the SDK, which should help address some concerns around tree shakability and bundle.

Closing this issue for repo maintenance, but if there are any other performance concerns, please let us know!