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

Remove macOS display refresh rate support #2628

Merged
merged 10 commits into from
Feb 6, 2025

Conversation

denrase
Copy link
Collaborator

@denrase denrase commented Jan 29, 2025

📜 Description

We can't 100% reliably detect the display refresh rate on macOS (mulidisplay). Also, dropped frames might be due to many other reasons on the host system, not just the apps performance. Therefore we decided to remove refresh rate support.

💡 Motivation and Context

Closes #2301

💚 How did you test it?

📝 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

@denrase denrase changed the title Remove macos display refresh rate Remove macos display refresh rate support Jan 29, 2025
@denrase denrase changed the title Remove macos display refresh rate support Remove macOS display refresh rate support Jan 29, 2025
Copy link
Contributor

github-actions bot commented Jan 29, 2025

Android Performance metrics 🚀

  Plain With Sentry Diff
Startup time 476.90 ms 588.56 ms 111.66 ms
Size 6.46 MiB 7.48 MiB 1.02 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
72dfc83 298.62 ms 340.14 ms 41.52 ms
a5031f1 379.02 ms 443.12 ms 64.10 ms
547db82 453.40 ms 482.88 ms 29.48 ms
7f14ddd 428.38 ms 511.90 ms 83.51 ms
d0476e1 412.20 ms 492.62 ms 80.42 ms
09c1f55 449.98 ms 509.38 ms 59.40 ms
0118295 365.71 ms 438.56 ms 72.85 ms
22ed6cb 326.27 ms 393.00 ms 66.73 ms
e964e2b 365.15 ms 431.00 ms 65.85 ms
dd25e43 449.40 ms 504.18 ms 54.78 ms

App size

Revision Plain With Sentry Diff
72dfc83 5.94 MiB 6.92 MiB 1001.71 KiB
a5031f1 6.33 MiB 7.29 MiB 987.32 KiB
547db82 6.49 MiB 7.56 MiB 1.07 MiB
7f14ddd 6.35 MiB 7.40 MiB 1.05 MiB
d0476e1 6.35 MiB 7.40 MiB 1.05 MiB
09c1f55 6.49 MiB 7.55 MiB 1.07 MiB
0118295 6.33 MiB 7.26 MiB 947.07 KiB
22ed6cb 6.06 MiB 7.03 MiB 993.37 KiB
e964e2b 6.33 MiB 7.28 MiB 962.81 KiB
dd25e43 6.46 MiB 7.48 MiB 1.02 MiB

Previous results on branch: enha/drop-macos-display-refresh-rate

Startup times

Revision Plain With Sentry Diff
2d36427 465.04 ms 553.77 ms 88.73 ms
c7ed76e 417.52 ms 502.73 ms 85.21 ms
d4a1dc8 468.37 ms 560.57 ms 92.20 ms

App size

Revision Plain With Sentry Diff
2d36427 6.46 MiB 7.48 MiB 1.02 MiB
c7ed76e 6.46 MiB 7.48 MiB 1.02 MiB
d4a1dc8 6.46 MiB 7.48 MiB 1.02 MiB

Copy link
Contributor

github-actions bot commented Jan 29, 2025

iOS Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1242.85 ms 1263.59 ms 20.74 ms
Size 8.42 MiB 9.91 MiB 1.48 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
9d43f71 1225.06 ms 1227.06 ms 2.00 ms
891efac 1233.78 ms 1248.31 ms 14.53 ms
0028e3e 1242.29 ms 1254.98 ms 12.69 ms
af2d175 1280.37 ms 1282.24 ms 1.88 ms
4d75417 1240.57 ms 1261.76 ms 21.19 ms
bd75526 1252.62 ms 1287.00 ms 34.38 ms
3e4b523 1260.53 ms 1270.06 ms 9.53 ms
e893df5 1247.90 ms 1262.31 ms 14.41 ms
dfcfde9 1238.83 ms 1255.30 ms 16.47 ms
633cf2e 1257.96 ms 1275.73 ms 17.77 ms

App size

Revision Plain With Sentry Diff
9d43f71 8.29 MiB 9.39 MiB 1.10 MiB
891efac 8.28 MiB 9.34 MiB 1.06 MiB
0028e3e 8.42 MiB 9.91 MiB 1.49 MiB
af2d175 8.15 MiB 9.12 MiB 986.22 KiB
4d75417 8.38 MiB 9.71 MiB 1.34 MiB
bd75526 8.38 MiB 9.76 MiB 1.39 MiB
3e4b523 8.28 MiB 9.33 MiB 1.05 MiB
e893df5 8.09 MiB 9.07 MiB 1001.04 KiB
dfcfde9 8.42 MiB 9.91 MiB 1.49 MiB
633cf2e 8.15 MiB 9.12 MiB 986.26 KiB

Previous results on branch: enha/drop-macos-display-refresh-rate

Startup times

Revision Plain With Sentry Diff
2d36427 1263.14 ms 1281.24 ms 18.10 ms
c7ed76e 1246.40 ms 1262.22 ms 15.83 ms
d4a1dc8 1252.42 ms 1270.27 ms 17.85 ms

App size

Revision Plain With Sentry Diff
2d36427 8.42 MiB 9.91 MiB 1.49 MiB
c7ed76e 8.42 MiB 9.91 MiB 1.49 MiB
d4a1dc8 8.42 MiB 9.91 MiB 1.49 MiB

@denrase denrase marked this pull request as ready for review January 29, 2025 12:52
Copy link

codecov bot commented Feb 3, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.31%. Comparing base (d84010d) to head (69ad7b9).
Report is 2 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (d84010d) and HEAD (69ad7b9). Click for more details.

HEAD has 8 uploads less than BASE
Flag BASE (d84010d) HEAD (69ad7b9)
9 1
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #2628       +/-   ##
===========================================
- Coverage   89.00%   63.31%   -25.70%     
===========================================
  Files         262        4      -258     
  Lines        8906      169     -8737     
===========================================
- Hits         7927      107     -7820     
+ Misses        979       62      -917     

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

CHANGELOG.md Show resolved Hide resolved
@buenaflor
Copy link
Contributor

looks good! I'll merge this after we did today's release

@denrase denrase requested a review from buenaflor February 5, 2025 12:12
@denrase denrase enabled auto-merge (squash) February 5, 2025 12:12
@denrase denrase merged commit b39b48c into main Feb 6, 2025
8 of 12 checks passed
@denrase denrase deleted the enha/drop-macos-display-refresh-rate branch February 6, 2025 10:02
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