Skip to content

RUM-4561 Delegate Drawable copy to background thread#2048

Merged
xgouchet merged 2 commits into
developfrom
xgouchet/sr_performance_mutable_list
May 23, 2024
Merged

RUM-4561 Delegate Drawable copy to background thread#2048
xgouchet merged 2 commits into
developfrom
xgouchet/sr_performance_mutable_list

Conversation

@xgouchet

Copy link
Copy Markdown
Contributor

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.

@xgouchet
xgouchet requested review from a team as code owners May 22, 2024 17:56
@codecov-commenter

codecov-commenter commented May 22, 2024

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.02%. Comparing base (daec9a0) to head (efb6200).

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     
Files Coverage Δ
...nreplay/internal/recorder/SessionReplayRecorder.kt 96.61% <100.00%> (+0.09%) ⬆️
...ssionreplay/internal/recorder/TreeViewTraversal.kt 95.92% <100.00%> (+0.80%) ⬆️
...roid/sessionreplay/internal/utils/DrawableUtils.kt 93.65% <100.00%> (ø)

... and 30 files with indirect coverage changes

}

val resolvedWireframes = internalLogger.measureMethodCallPerf(
javaClass,

Copy link
Copy Markdown
Member

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?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Makes sense indeeed

@xgouchet xgouchet changed the title Delegate Drawable copy to background thread RUM-4561 Delegate Drawable copy to background thread May 23, 2024
@xgouchet xgouchet mentioned this pull request May 23, 2024
3 tasks
override fun onSuccess(bitmap: Bitmap) {
mainThreadHandler.post {
@Suppress("ThreadSafety") // this runs on the main thread
executorService.submitSafe("drawOnCanvas", internalLogger) {

Copy link
Copy Markdown
Member

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?

Copy link
Copy Markdown
Contributor Author

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.

Copy link
Copy Markdown
Contributor Author

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 👍

@xgouchet
xgouchet merged commit de8989f into develop May 23, 2024
@xgouchet
xgouchet deleted the xgouchet/sr_performance_mutable_list branch May 23, 2024 08:53
@xgouchet xgouchet added this to the 2.11.x milestone Jul 31, 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.

4 participants