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

fix(ember): Disable performance in FastBoot #7282

Merged
merged 1 commit into from Feb 27, 2023

Conversation

mydea
Copy link
Member

@mydea mydea commented Feb 27, 2023

We do not want to run the instance initializer in fastboot env, as that relies on Browser stuff.

Note that it would prob. make sense to skip all of sentry in fastboot (SSR), but that probably has to happen in user code.
This fix at least should lead to the app not failing to be served when fastboot & sentry are installed.

ref #7258

We do not want to run the instance initializer in fastboot env, as that relies on Browser stuff.
@mydea mydea self-assigned this Feb 27, 2023
@mydea mydea mentioned this pull request Feb 27, 2023
3 tasks
Copy link
Member

@Lms24 Lms24 left a comment

Choose a reason for hiding this comment

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

TIL about Ember fastboot

Makes sense to me

@github-actions
Copy link
Contributor

Replay SDK metrics 🚀

    Plain +Sentry +Replay
Revision Value Value Diff Ratio Value Diff Ratio
LCP This PR 80cb7a5 76.14 ms 95.76 ms +19.62 ms +25.76 % 130.24 ms +54.10 ms +71.05 %
Previous 4b95c04 74.67 ms 100.14 ms +25.47 ms +34.11 % 132.23 ms +57.56 ms +77.09 %
CLS This PR 80cb7a5 0.06 ms 0.06 ms -0.00 ms -0.36 % 0.06 ms -0.00 ms -0.31 %
Previous 4b95c04 0.06 ms 0.06 ms -0.00 ms -0.79 % 0.06 ms -0.00 ms -0.47 %
CPU This PR 80cb7a5 14.33 % 14.46 % +0.13 pp +0.89 % 20.59 % +6.25 pp +43.61 %
Previous 4b95c04 12.32 % 12.48 % +0.16 pp +1.28 % 20.27 % +7.94 pp +64.46 %
JS heap avg This PR 80cb7a5 1.94 MB 1.99 MB +54.96 kB +2.83 % 2.87 MB +928.52 kB +47.87 %
Previous 4b95c04 1.94 MB 1.99 MB +43.74 kB +2.25 % 2.86 MB +920.88 kB +47.38 %
JS heap max This PR 80cb7a5 2.31 MB 2.57 MB +264.19 kB +11.46 % 3.38 MB +1.07 MB +46.50 %
Previous 4b95c04 2.3 MB 2.55 MB +249.86 kB +10.85 % 3.35 MB +1.05 MB +45.58 %
netTx This PR 80cb7a5 0 B 0 B 0 B n/a 2.21 kB +2.21 kB n/a
Previous 4b95c04 0 B 0 B 0 B n/a 2.21 kB +2.21 kB n/a
netRx This PR 80cb7a5 0 B 0 B 0 B n/a 41 B +41 B n/a
Previous 4b95c04 0 B 0 B 0 B n/a 41 B +41 B n/a
netCount This PR 80cb7a5 0 0 0 n/a 1 +1 n/a
Previous 4b95c04 0 0 0 n/a 1 +1 n/a
netTime This PR 80cb7a5 0.00 ms 0.00 ms 0.00 ms n/a 89.78 ms +89.78 ms n/a
Previous 4b95c04 0.00 ms 0.00 ms 0.00 ms n/a 90.32 ms +90.32 ms n/a

Previous results on branch: develop

RevisionLCPCLSCPUJS heap avgJS heap maxnetTxnetRxnetCountnetTime
4b95c04+57.56 ms-0.00 ms+7.94 pp+920.88 kB+1.05 MB+2.21 kB+41 B+1+90.32 ms
e60cd02+56.25 ms-0.00 ms+6.32 pp+927.44 kB+1.06 MB+2.21 kB+41 B+1+117.55 ms
e25c067+48.34 ms+0.00 ms+5.59 pp+926.37 kB+1.05 MB+2.22 kB+41 B+1+65.23 ms
b1b249b+43.88 ms+0.00 ms+4.80 pp+937.99 kB+1.05 MB+2.22 kB+41 B+1+111.56 ms
12e34d4+28.57 ms+0.00 ms+5.77 pp+930.12 kB+1.04 MB+2.26 kB+41 B+1+109.67 ms
c46c56c+65.45 ms-0.00 ms+5.38 pp+930.26 kB+1.07 MB+2.21 kB+41 B+1+91.29 ms
7f4c4ec+56.64 ms-0.00 ms+5.57 pp+927.42 kB+1.06 MB+2.21 kB+41 B+1+110.83 ms
00d2360+55.18 ms+0.00 ms+2.23 pp+934.14 kB+1.05 MB+2.22 kB+41 B+1+71.65 ms

*) pp - percentage points - an absolute difference between two percentages.
Last updated: Mon, 27 Feb 2023 13:51:43 GMT

@k-fish
Copy link
Member

k-fish commented Feb 27, 2023

@mydea sounds good. I suspect we will eventually want SSR similar to nextjs for all our js sdk's since it's part of the performance lifetime of the user action still (and ssr can be where all the slowness lies), but if fastboot wasn't working anyway then I'm good to disable it 👍

@mydea
Copy link
Member Author

mydea commented Feb 27, 2023

@mydea sounds good. I suspect we will eventually want SSR similar to nextjs for all our js sdk's since it's part of the performance lifetime of the user action still (and ssr can be where all the slowness lies), but if fastboot wasn't working anyway then I'm good to disable it 👍

Yes, there is def. more that we can do here, but this should at least unblock this for now :)

@mydea mydea merged commit 3324324 into develop Feb 27, 2023
@mydea mydea deleted the fn/disable-sentry-fastboot branch February 27, 2023 14:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants