Skip to content

feat(replay): Use requestIdleCallback for events - #7191

Closed
mydea wants to merge 1 commit into
developfrom
fn/replay-requestIdleCallback
Closed

feat(replay): Use requestIdleCallback for events#7191
mydea wants to merge 1 commit into
developfrom
fn/replay-requestIdleCallback

Conversation

@mydea

@mydeamydea commented Feb 15, 2023

Copy link
Copy Markdown
Member

This PR updates the event handling of replay to use window.requestIdleCallback, if available.
This API is supported everywhere except for Safari, and should schedule a callback to be run when the browser has "resources" for it.

Hopefully, this should improve performance in scenarios where a lot of events are emitted.

I defined a max. timeout of 50ms, meaning that after max. 50ms each callback will be executed anyhow. This could be tweaked, but feels like a reasonably defensive value here. If the API is not available, we just run the callbacks immediately (like now), so it should degrade gracefully.

Note that there would also be an approach to only have a single callback that keeps an internal queue, but that would require more work to ensure that events cannot be postponed for too long, so IMHO this is the easier solution.

I tried this in my super simple test app, where it works as expected. it would be nice though to test this in a more complex app...

Note: I also refactored the addUpdate method to split this into dedicated methods, as this was a bit mixed up right now. Now, we have a dedicated (private) method for the event emitting, and one for adding a "generic" event from other handlers etc.

See https://w3c.github.io/requestidlecallback/ for some more info.

ref #6946
ref #7085

@github-actions

Copy link
Copy Markdown
Contributor

size-limit report 📦

PathSize
@sentry/browser - ES5 CDN Bundle (gzipped + minified)20.09 KB (+0.01% 🔺)
@sentry/browser - ES5 CDN Bundle (minified)62.23 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped + minified)18.71 KB (0%)
@sentry/browser - ES6 CDN Bundle (minified)55.37 KB (0%)
@sentry/browser - Webpack (gzipped + minified)20.44 KB (0%)
@sentry/browser - Webpack (minified)66.81 KB (0%)
@sentry/react - Webpack (gzipped + minified)20.47 KB (0%)
@sentry/nextjs Client - Webpack (gzipped + minified)47.9 KB (0%)
@sentry/browser + @sentry/tracing - ES5 CDN Bundle (gzipped + minified)27.03 KB (+0.01% 🔺)
@sentry/browser + @sentry/tracing - ES6 CDN Bundle (gzipped + minified)25.29 KB (0%)
@sentry/replay ES6 CDN Bundle (gzipped + minified)42.57 KB (+0.13% 🔺)
@sentry/replay - Webpack (gzipped + minified)36.78 KB (+0.16% 🔺)
@sentry/browser + @sentry/tracing + @sentry/replay - ES6 CDN Bundle (gzipped + minified)60.29 KB (+0.09% 🔺)
@sentry/browser + @sentry/replay - ES6 CDN Bundle (gzipped + minified)53.83 KB (+0.11% 🔺)

@github-actions

Copy link
Copy Markdown
Contributor

Replay SDK metrics 🚀

Plain+Sentry+Replay
RevisionValueValueDiffRatioValueDiffRatio
LCPThis PR b458a65109.01 ms136.56 ms+27.55 ms+25.27 %176.50 ms+67.49 ms+61.91 %
Baseline 4c07d0295.26 ms129.60 ms+34.34 ms+36.05 %163.27 ms+68.01 ms+71.39 %
CLSThis PR b458a650.06 ms0.06 ms+0.00 ms+0.01 %0.06 ms+0.00 ms+0.14 %
Baseline 4c07d020.06 ms0.06 ms+0.00 ms+0.00 %0.06 ms+0.00 ms+0.05 %
CPUThis PR b458a6523.32 %24.07 %+0.75 pp+3.20 %33.59 %+10.27 pp+44.05 %
Baseline 4c07d0222.20 %22.83 %+0.62 pp+2.80 %29.77 %+7.57 pp+34.09 %
JS heap avgThis PR b458a651.94 MB2 MB+56.85 kB+2.93 %2.87 MB+927.53 kB+47.83 %
Baseline 4c07d021.93 MB1.99 MB+59.82 kB+3.09 %2.85 MB+918.59 kB+47.51 %
JS heap maxThis PR b458a652.32 MB2.56 MB+236.76 kB+10.20 %3.36 MB+1.04 MB+44.72 %
Baseline 4c07d022.31 MB2.56 MB+242.26 kB+10.47 %3.35 MB+1.03 MB+44.66 %
netTxThis PR b458a650 B0 B0 Bn/a2.4 kB+2.4 kBn/a
Baseline 4c07d020 B0 B0 Bn/a2.22 kB+2.22 kBn/a
netRxThis PR b458a650 B0 B0 Bn/a41 B+41 Bn/a
Baseline 4c07d020 B0 B0 Bn/a41 B+41 Bn/a
netCountThis PR b458a65000n/a1+1n/a
Baseline 4c07d02000n/a1+1n/a
netTimeThis PR b458a650.00 ms0.00 ms0.00 msn/a109.82 ms+109.82 msn/a
Baseline 4c07d020.00 ms0.00 ms0.00 msn/a127.76 ms+127.76 msn/a

Baseline results on branch: develop

RevisionLCPCLSCPUJS heap avgJS heap maxnetTxnetRxnetCountnetTime
1c9ed4f+50.07 ms-0.00 ms+6.39 pp+928.84 kB+1.05 MB+2.2 kB+41 B+1+70.97 ms
1921888+55.79 ms+0.00 ms+5.50 pp+926.71 kB+1.06 MB+2.23 kB+41 B+1+79.78 ms
e9eec27+61.38 ms-0.00 ms+6.04 pp+927.84 kB+1.05 MB+2.21 kB+41 B+1+88.58 ms
d604022+58.83 ms-0.00 ms+7.65 pp+930.16 kB+1.05 MB+2.21 kB+41 B+1+109.63 ms
a961e57+54.75 ms-0.00 ms+6.50 pp+929.18 kB+1.07 MB+2.21 kB+41 B+1+92.73 ms
f7c0a2f+46.14 ms+0.00 ms+6.37 pp+921.47 kB+1.06 MB+2.23 kB+41 B+1+207.30 ms
cb19818+57.16 ms+0.00 ms+11.95 pp+1.07 MB+2.21 MB+2.52 kB+41 B+1+111.50 ms
ee301c3+71.07 ms-0.00 ms+12.64 pp+1.07 MB+2.22 MB+2.55 kB+41 B+1+94.67 ms
93c4759+61.10 ms-0.00 ms+12.72 pp+1.08 MB+2.19 MB+2.57 kB+41 B+1+116.75 ms

*) pp - percentage points - an absolute difference between two percentages.
Last updated: Wed, 15 Feb 2023 11:25:05 GMT

@billyvgbillyvg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's hold off on this change for now unless we can show some measureable improvements

@mydea

Copy link
Copy Markdown
MemberAuthor

Let's hold off on this change for now unless we can show some measureable improvements

Yes, definitely!

@mydea

Copy link
Copy Markdown
MemberAuthor

Closing this for now, as we're going to explore a different approach - see getsentry/rrweb#58

@mydeamydea closed this Feb 27, 2023
@mydea
mydea deleted the fn/replay-requestIdleCallback branch December 3, 2024 09:31
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@mydea@billyvg@AbhiPrasad