-
Notifications
You must be signed in to change notification settings - Fork 61
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
RUM-4561 Delegate Drawable copy to background thread #2048
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #2048 +/- ##
===========================================
- Coverage 83.10% 83.02% -0.09%
===========================================
Files 494 494
Lines 17720 17688 -32
Branches 2686 2684 -2
===========================================
- Hits 14726 14684 -42
- Misses 2253 2259 +6
- Partials 741 745 +4
|
@@ -76,6 +81,14 @@ internal class TreeViewTraversal( | |||
) | |||
} | |||
|
|||
val resolvedWireframes = internalLogger.measureMethodCallPerf( | |||
javaClass, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to what was done with WindowsOnDrawListener, do we need to also add this class to proguard rules to prevent classname obfuscation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense indeeed
@@ -59,8 +58,8 @@ internal class DrawableUtils( | |||
resizeBitmapCallback = object : | |||
ResizeBitmapCallback { | |||
override fun onSuccess(bitmap: Bitmap) { | |||
mainThreadHandler.post { | |||
@Suppress("ThreadSafety") // this runs on the main thread | |||
executorService.submitSafe("drawOnCanvas", internalLogger) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it safe to operate with bitmap/canvas from the worker thread on all Android versions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the tests I've done, it seems so. I'll do more test to ensure it works on all levels.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does work on all versions 👍
What does this PR do?
Our previous mechanism of copying a drawable's content was performed on the main thread.
When starting an app, this would create a lot of main thread task to run. And any new image would also take some of the main thread time, creating a lot of jank frames in any non hello world app.
This will fix the performance issue some of our customers started to notice when enabling SR.