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: Too long flush duration #2370

Merged
merged 11 commits into from
Nov 10, 2022
Merged

fix: Too long flush duration #2370

merged 11 commits into from
Nov 10, 2022

Conversation

philipphofmann
Copy link
Member

@philipphofmann philipphofmann commented Nov 8, 2022

📜 Description

Move the calculation of the flush dispatch time to the beginning of the flush function to avoid any code until the call to dispatch_group_wait adds up to the total flush duration.

💡 Motivation and Context

Fixes GH-2334, GH-2340

💚 How did you test it?

Unit tests, and GH actions dummy matrix.

📝 Checklist

  • I reviewed the submitted code
  • I added tests to verify the changes
  • I updated the docs if needed
  • Review from the native team if needed
  • No breaking changes

🔮 Next steps

Move calculation of the flush dispatch time to the beginning of the
flush function to avoid any code until the call to dispatch_group_wait
adds up to the total flush duration.

Fixes GH-2334, GH-2340
@github-actions
Copy link

github-actions bot commented Nov 10, 2022

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1213.49 ms 1229.54 ms 16.05 ms
Size 20.75 KiB 368.01 KiB 347.26 KiB

Baseline results on branch: master

Startup times

Revision Plain With Sentry Diff
c929040 1254.84 ms 1278.42 ms 23.58 ms
e43ce74 1235.77 ms 1252.06 ms 16.29 ms
0979ac6 1217.96 ms 1244.88 ms 26.92 ms
7d746c3 1220.78 ms 1249.47 ms 28.69 ms
202334c 1265.41 ms 1277.34 ms 11.93 ms
64225be 1213.96 ms 1227.62 ms 13.66 ms
d883642 1215.39 ms 1242.78 ms 27.39 ms
d47c2c0 1264.73 ms 1271.88 ms 7.15 ms
8607e67 1255.80 ms 1259.50 ms 3.70 ms
0fdf0b2 1194.37 ms 1227.90 ms 33.53 ms

App size

Revision Plain With Sentry Diff
c929040 20.51 KiB 333.10 KiB 312.59 KiB
e43ce74 20.51 KiB 335.49 KiB 314.99 KiB
0979ac6 20.50 KiB 342.31 KiB 321.81 KiB
7d746c3 20.50 KiB 343.46 KiB 322.96 KiB
202334c 20.51 KiB 331.79 KiB 311.28 KiB
64225be 20.50 KiB 339.02 KiB 318.52 KiB
d883642 20.50 KiB 338.99 KiB 318.49 KiB
d47c2c0 20.50 KiB 342.31 KiB 321.80 KiB
8607e67 20.50 KiB 338.99 KiB 318.49 KiB
0fdf0b2 20.51 KiB 332.90 KiB 312.39 KiB

Previous results on branch: fix/flush-duration

Startup times

Revision Plain With Sentry Diff
eb6e60b 1231.27 ms 1256.12 ms 24.85 ms
eb8008d 1201.45 ms 1228.34 ms 26.89 ms
d2323d1 1223.55 ms 1249.10 ms 25.55 ms
25606a4 1205.26 ms 1229.68 ms 24.42 ms
7ed4aef 1228.40 ms 1241.92 ms 13.52 ms

App size

Revision Plain With Sentry Diff
eb6e60b 20.75 KiB 368.02 KiB 347.26 KiB
eb8008d 20.75 KiB 368.02 KiB 347.26 KiB
d2323d1 20.75 KiB 368.01 KiB 347.26 KiB
25606a4 20.75 KiB 368.02 KiB 347.26 KiB
7ed4aef 20.75 KiB 368.02 KiB 347.26 KiB

@philipphofmann philipphofmann merged commit e2a3f3e into master Nov 10, 2022
@philipphofmann philipphofmann deleted the fix/flush-duration branch November 10, 2022 14:35
kevinrenskers added a commit that referenced this pull request Nov 14, 2022
* master:
  Report pre-warmed app starts (#1969)
  fix: Too long flush duration (#2370)
  release: 7.30.2
  fix: profile payload issues (#2375)
kevinrenskers added a commit that referenced this pull request Nov 14, 2022
…ents

* master:
  feat: Store breadcrumbs to disk for OOM events (#2347)
  Report pre-warmed app starts (#1969)
  fix: Too long flush duration (#2370)
  release: 7.30.2
  fix: profile payload issues (#2375)
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.

Fix flaky SentryHttpTransportTests.testFlush_BlocksCallingThread_TimesOut
2 participants