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

Reduce main thread work on init #3036

Merged
merged 15 commits into from
Nov 27, 2023
Merged

Reduce main thread work on init #3036

merged 15 commits into from
Nov 27, 2023

Conversation

stefanosiano
Copy link
Member

📜 Description

Moved old profiles deletion to the background
moved some Integration.register() to the background using executorService
added tests

💡 Motivation and Context

This is an attempt to improve SDK startup timing.
Relates to #2222

💚 How did you test it?

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

Copy link
Contributor

github-actions bot commented Nov 7, 2023

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 386.71 ms 465.34 ms 78.63 ms
Size 1.72 MiB 2.29 MiB 579.13 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
0404ea3 332.47 ms 401.12 ms 68.66 ms
86f0096 371.86 ms 439.78 ms 67.92 ms
99a51e2 405.11 ms 479.65 ms 74.54 ms
4e29063 384.14 ms 447.74 ms 63.60 ms
93a76ca 377.41 ms 448.22 ms 70.81 ms
2fad834 390.07 ms 470.80 ms 80.73 ms
4bf95dd 345.96 ms 414.24 ms 68.28 ms
0404ea3 394.73 ms 461.79 ms 67.06 ms
0bd723b 395.44 ms 472.66 ms 77.22 ms
bc4be3b 360.40 ms 435.04 ms 74.64 ms

App size

Revision Plain With Sentry Diff
0404ea3 1.72 MiB 2.29 MiB 577.52 KiB
86f0096 1.72 MiB 2.29 MiB 576.50 KiB
99a51e2 1.72 MiB 2.29 MiB 576.34 KiB
4e29063 1.72 MiB 2.29 MiB 578.38 KiB
93a76ca 1.72 MiB 2.29 MiB 576.75 KiB
2fad834 1.72 MiB 2.29 MiB 577.53 KiB
4bf95dd 1.72 MiB 2.29 MiB 576.40 KiB
0404ea3 1.72 MiB 2.29 MiB 577.52 KiB
0bd723b 1.72 MiB 2.29 MiB 578.09 KiB
bc4be3b 1.72 MiB 2.29 MiB 576.53 KiB

Previous results on branch: fix/reduce-io-init

Startup times

Revision Plain With Sentry Diff
2462226 400.98 ms 477.18 ms 76.20 ms
077a103 410.41 ms 464.64 ms 54.24 ms
5295259 376.14 ms 448.89 ms 72.75 ms
5c39a33 294.54 ms 327.67 ms 33.12 ms
644ca6a 448.52 ms 547.22 ms 98.70 ms

App size

Revision Plain With Sentry Diff
2462226 1.72 MiB 2.29 MiB 578.43 KiB
077a103 1.72 MiB 2.29 MiB 578.27 KiB
5295259 1.72 MiB 2.29 MiB 577.94 KiB
5c39a33 1.72 MiB 2.29 MiB 579.27 KiB
644ca6a 1.72 MiB 2.29 MiB 578.27 KiB

@stefanosiano stefanosiano marked this pull request as ready for review November 8, 2023 10:33
Copy link
Member

@markushi markushi left a comment

Choose a reason for hiding this comment

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

Looking good, I left a few comments which need to be addressed before merging. 🚀

Copy link
Member

@markushi markushi left a comment

Choose a reason for hiding this comment

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

LGTM!

CHANGELOG.md Outdated Show resolved Hide resolved
* moved several integrations to the background to reduce main thread usage on SDK init:
-AnrIntegration
-PhoneStateBreadcrumbsIntegration
-SystemEventsBreadcrumbsIntegration
-TempSensorBreadcrumbsIntegration
* added synchronization between register() and close() methods of Integrations
* created a DeferredExecutorService for tests
@stefanosiano stefanosiano merged commit e6ffd7b into main Nov 27, 2023
20 checks passed
@stefanosiano stefanosiano deleted the fix/reduce-io-init branch November 27, 2023 18:31
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.

None yet

3 participants