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

Startup improvements #2089

Merged
merged 7 commits into from
Nov 14, 2024
Merged

Startup improvements #2089

merged 7 commits into from
Nov 14, 2024

Conversation

lemnik
Copy link
Contributor

@lemnik lemnik commented Oct 8, 2024

Goal

Improve the startup threading behavior of the SDK to reduce the chances of deadlocks & ANRs.

Design

Previously the SDK startup included several calls which moved work to a background thread, and then used Future.get() to block until the work was complete. This was done to keep I/O operations (and similar) off the main thread, but introduced the cross-thread overheads without any direct benefit (all the cost of a thread, without actually doing anything in parallel).

This PR introduces a new model based around a Provider interface, which is similar to FutureTask in concept. The primary implementation is RunnableProvider which has a very primitive work-stealing ability to ensure that Providers do not block each other (avoiding possible deadlocks where tasks are blocked waiting for tasks on the same queue) unless required. That is: a Provider will typically try and "do" another providers work (unless that provider has already done, or is busy doing said work).

As far as possible the new model does not block until the data from a Provider is actually required by a non-provider (such as the NDK module, or a crash handler). This should allow better use of the threads available during startup.

Testing

Modified the existing unit tests to handle the changes to the models.

@bugsnagbot
Copy link
Collaborator

bugsnagbot commented Oct 8, 2024

Android notifier sizes

Format Size impact of Bugsnag (kB) Size impact of Bugsnag when Minified (kB)
APK 1856.29 1675.76
arm64_v8a 635.14 450.82
armeabi_v7a 569.61 385.29
x86 708.85 528.62
x86_64 680.18 495.86

Generated by 🚫 Danger

Copy link
Contributor

@YYChen01988 YYChen01988 left a comment

Choose a reason for hiding this comment

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

LGTM, pending changelog entrance

@lemnik lemnik merged commit e7f307c into next Nov 14, 2024
57 checks passed
@lemnik lemnik deleted the integration/startup-improvements branch November 14, 2024 11:50
@YYChen01988 YYChen01988 mentioned this pull request Nov 14, 2024
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.

3 participants