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

Open
wants to merge 4 commits into
base: next
Choose a base branch
from
Open

Startup improvements #2089

wants to merge 4 commits into from

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

Android notifier sizes

Format Size impact of Bugsnag (kB) Size impact of Bugsnag when Minified (kB)
APK 1861.31 1675.2
arm64_v8a 639.23 450.82
armeabi_v7a 573.7 385.29
x86 717.04 528.62
x86_64 684.28 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

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