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

Fallback to main window and main screen for macOS display refresh rate #2621

Closed
wants to merge 6 commits into from

Conversation

denrase
Copy link
Collaborator

@denrase denrase commented Jan 27, 2025

📜 Description

Fallback to main window and main screen for macOS display refresh rate, as NSApplication.shared.keyWindow might be null when we load the screen refresh rate.

💡 Motivation and Context

Closes #2301
Relates to #2299

💚 How did you test it?

Ran sample app on macOS.

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPii is enabled
  • I updated the docs if needed
  • All tests passing
  • No breaking changes

Copy link

codecov bot commented Jan 27, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.53%. Comparing base (f85b52d) to head (300d55d).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2621      +/-   ##
==========================================
+ Coverage   89.10%   92.53%   +3.42%     
==========================================
  Files         263       91     -172     
  Lines        8943     2986    -5957     
==========================================
- Hits         7969     2763    -5206     
+ Misses        974      223     -751     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@denrase denrase marked this pull request as ready for review January 27, 2025 15:34
Copy link
Contributor

github-actions bot commented Jan 27, 2025

Android Performance metrics 🚀

  Plain With Sentry Diff
Startup time 732.67 ms 855.57 ms 122.90 ms
Size 6.46 MiB 7.48 MiB 1.02 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
b930b94 484.42 ms 525.51 ms 41.09 ms
d8519f9 352.83 ms 420.46 ms 67.62 ms
7ade5af 341.04 ms 386.84 ms 45.80 ms
73d70bf 467.08 ms 546.12 ms 79.04 ms
299a3f6 508.79 ms 563.71 ms 54.93 ms
8776cdf 353.32 ms 416.58 ms 63.26 ms
e893df5 310.60 ms 380.58 ms 69.98 ms
0764150 469.49 ms 541.86 ms 72.37 ms
09c1f55 449.98 ms 509.38 ms 59.40 ms
f2db4ec 372.46 ms 469.72 ms 97.26 ms

App size

Revision Plain With Sentry Diff
b930b94 6.49 MiB 7.57 MiB 1.08 MiB
d8519f9 6.33 MiB 7.26 MiB 946.13 KiB
7ade5af 5.94 MiB 6.95 MiB 1.01 MiB
73d70bf 6.52 MiB 7.59 MiB 1.06 MiB
299a3f6 6.49 MiB 7.56 MiB 1.07 MiB
8776cdf 6.34 MiB 7.28 MiB 966.66 KiB
e893df5 6.06 MiB 7.09 MiB 1.03 MiB
0764150 6.46 MiB 7.48 MiB 1.01 MiB
09c1f55 6.49 MiB 7.55 MiB 1.07 MiB
f2db4ec 6.06 MiB 7.03 MiB 990.27 KiB

Previous results on branch: fix/fallback-to-main-screen-macos

Startup times

Revision Plain With Sentry Diff
8d40739 437.30 ms 518.57 ms 81.27 ms
41cd4ac 447.35 ms 512.46 ms 65.11 ms

App size

Revision Plain With Sentry Diff
8d40739 6.46 MiB 7.48 MiB 1.02 MiB
41cd4ac 6.46 MiB 7.48 MiB 1.02 MiB

Copy link
Contributor

github-actions bot commented Jan 27, 2025

iOS Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1234.59 ms 1257.83 ms 23.24 ms
Size 8.42 MiB 9.91 MiB 1.49 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
8bac8ba 1261.53 ms 1279.67 ms 18.14 ms
30c1193 1227.20 ms 1235.00 ms 7.80 ms
3a4de05 1243.07 ms 1265.91 ms 22.85 ms
2e93bab 1237.08 ms 1258.41 ms 21.33 ms
31b2afb 1244.90 ms 1261.37 ms 16.47 ms
be08ed1 1247.12 ms 1272.19 ms 25.07 ms
cdf7172 1264.43 ms 1291.39 ms 26.96 ms
103eb14 1254.11 ms 1280.65 ms 26.55 ms
daa1b33 1224.41 ms 1244.59 ms 20.18 ms
5aba417 1265.31 ms 1287.90 ms 22.59 ms

App size

Revision Plain With Sentry Diff
8bac8ba 8.42 MiB 9.89 MiB 1.47 MiB
30c1193 8.28 MiB 9.34 MiB 1.06 MiB
3a4de05 8.42 MiB 9.88 MiB 1.46 MiB
2e93bab 8.38 MiB 9.73 MiB 1.36 MiB
31b2afb 8.33 MiB 9.40 MiB 1.07 MiB
be08ed1 8.32 MiB 9.38 MiB 1.06 MiB
cdf7172 8.16 MiB 9.16 MiB 1.01 MiB
103eb14 8.38 MiB 9.71 MiB 1.34 MiB
daa1b33 8.28 MiB 9.34 MiB 1.05 MiB
5aba417 8.16 MiB 9.17 MiB 1.01 MiB

Previous results on branch: fix/fallback-to-main-screen-macos

Startup times

Revision Plain With Sentry Diff
41cd4ac 1243.71 ms 1251.62 ms 7.91 ms
8d40739 1251.21 ms 1266.77 ms 15.56 ms

App size

Revision Plain With Sentry Diff
41cd4ac 8.42 MiB 9.91 MiB 1.49 MiB
8d40739 8.42 MiB 9.91 MiB 1.49 MiB

@denrase denrase enabled auto-merge (squash) January 27, 2025 16:16
@denrase denrase requested a review from buenaflor January 27, 2025 16:16
@denrase denrase requested a review from buenaflor January 28, 2025 14:24
@buenaflor
Copy link
Contributor

as discussed, we will drop support for fetching refresh rate on desktop

@denrase denrase closed this Jan 29, 2025
auto-merge was automatically disabled January 29, 2025 12:52

Pull request was closed

Sign up for free to 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.

_native?.displayRefreshRate() returns 0 on macOS Monterey and below
2 participants