Skip to content

REPLAY-1890: Implement pool of reusable bitmaps#1554

Merged
jonathanmos merged 1 commit into
feature/base64-jmoskovichfrom
jmoskovich/replay-1890/bitmap-pool
Aug 9, 2023
Merged

REPLAY-1890: Implement pool of reusable bitmaps#1554
jonathanmos merged 1 commit into
feature/base64-jmoskovichfrom
jmoskovich/replay-1890/bitmap-pool

Conversation

@jonathanmos

@jonathanmos jonathanmos commented Jul 25, 2023

Copy link
Copy Markdown
Member

What does this PR do?

Adds a reusable bitmap pool for base64.

Motivation

Reduce the number of new bitmaps created.

Additional Notes

Anything else we should know when reviewing?

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Make sure you discussed the feature or bugfix with the maintaining team in an Issue
  • Make sure each commit and the PR mention the Issue number (cf the CONTRIBUTING doc)

@jonathanmos
jonathanmos force-pushed the jmoskovich/replay-1890/bitmap-pool branch from bd8e3f1 to f703ef9 Compare July 25, 2023 07:37
@codecov-commenter

codecov-commenter commented Jul 25, 2023 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 65.64885% with 45 lines in your changes missing coverage. Please review.

Project coverage is 83.32%. Comparing base (dcd6805) to head (227afe9).

❗ Current head 227afe9 differs from pull request most recent head 8002635

Please upload reports for the commit 8002635 to get more accurate results.

Files with missing lines Patch % Lines
...ssionreplay/internal/recorder/base64/BitmapPool.kt 61.97% 23 Missing and 4 partials ⚠️
...android/sessionreplay/internal/utils/CacheUtils.kt 0.00% 11 Missing ⚠️
...id/sessionreplay/internal/recorder/base64/Cache.kt 25.00% 3 Missing ⚠️
...nreplay/internal/recorder/base64/Base64LRUCache.kt 33.33% 2 Missing ⚠️
...ndroid/sessionreplay/internal/utils/Base64Utils.kt 0.00% 1 Missing ⚠️
...roid/sessionreplay/internal/utils/DrawableUtils.kt 92.86% 0 Missing and 1 partial ⚠️
Additional details and impacted files 
@@                      Coverage Diff                      @@
##           feature/base64-jmoskovich    #1554      +/-   ##
=============================================================
- Coverage                      83.63%   83.32%   -0.30%     
=============================================================
  Files                            439      443       +4     
  Lines                          14977    15070      +93     
  Branches                        2258     2272      +14     
=============================================================
+ Hits                           12525    12557      +32     
- Misses                          1879     1923      +44     
- Partials                         573      590      +17
Files with missing lines Coverage Δ
...nreplay/internal/async/RecordedDataQueueHandler.kt 88.52% <ø> (-0.87%) ⬇️
...eplay/internal/recorder/base64/Base64Serializer.kt 88.73% <100.00%> (+0.85%) ⬆️
...y/internal/recorder/base64/WebPImageCompression.kt 90.00% <100.00%> (ø)
...id/sessionreplay/internal/utils/InvocationUtils.kt 100.00% <100.00%> (ø)
...ndroid/sessionreplay/internal/utils/Base64Utils.kt 66.67% <0.00%> (ø)
...roid/sessionreplay/internal/utils/DrawableUtils.kt 96.97% <92.86%> (-3.03%) ⬇️
...nreplay/internal/recorder/base64/Base64LRUCache.kt 84.85% <33.33%> (+13.42%) ⬆️
...id/sessionreplay/internal/recorder/base64/Cache.kt 25.00% <25.00%> (ø)
...android/sessionreplay/internal/utils/CacheUtils.kt 0.00% <0.00%> (ø)
...ssionreplay/internal/recorder/base64/BitmapPool.kt 61.97% <61.97%> (ø)

... and 11 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

mariusc83
mariusc83 requested changes Jul 25, 2023
View reviewed changes
Comment thread ...src/main/kotlin/com/datadog/android/sessionreplay/internal/async/RecordedDataQueueHandler.kt Outdated
Comment thread ...lay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/base64/BitmapPool.kt Outdated
mariusc83
mariusc83 requested changes Jul 27, 2023
View reviewed changes

@mariusc83 mariusc83 left a comment

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.

I like the idea of backing up the BitmapPool with an LruCache. There are still things to address in here but the idea works.

Comment thread ...lay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/base64/BitmapPool.kt
Comment thread ...lay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/base64/BitmapPool.kt
Comment thread ...lay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/base64/BitmapPool.kt
Comment thread ...lay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/base64/BitmapPool.kt Outdated
Comment thread ...lay/src/main/kotlin/com/datadog/android/sessionreplay/internal/recorder/base64/BitmapPool.kt Outdated
Comment thread ...ion-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/utils/DrawableUtils.kt Outdated
Comment thread ...src/test/kotlin/com/datadog/android/sessionreplay/internal/recorder/base64/BitmapPoolTest.kt Outdated
Comment thread ...src/test/kotlin/com/datadog/android/sessionreplay/internal/recorder/base64/BitmapPoolTest.kt Outdated
Comment thread ...src/test/kotlin/com/datadog/android/sessionreplay/internal/recorder/base64/BitmapPoolTest.kt Outdated
Comment thread ...src/test/kotlin/com/datadog/android/sessionreplay/internal/recorder/base64/BitmapPoolTest.kt
@jonathanmos
jonathanmos force-pushed the jmoskovich/replay-1890/bitmap-pool branch 3 times, most recently from ab97424 to ba3545f Compare July 31, 2023 07:46
@jonathanmos
jonathanmos force-pushed the jmoskovich/replay-1890/bitmap-pool branch 2 times, most recently from 080e566 to c0a9969 Compare July 31, 2023 10:30
@jonathanmos
jonathanmos marked this pull request as ready for review July 31, 2023 11:00
@jonathanmos
jonathanmos requested a review from a team as a code owner July 31, 2023 11:00
mariusc83
mariusc83 previously approved these changes Aug 8, 2023
View reviewed changes
Comment thread features/dd-sdk-android-session-replay/build.gradle.kts Outdated
@jonathanmos
jonathanmos force-pushed the jmoskovich/replay-1890/bitmap-pool branch from c0a9969 to 227afe9 Compare August 8, 2023 08:35
@jonathanmos
jonathanmos force-pushed the jmoskovich/replay-1890/bitmap-pool branch from 227afe9 to 8002635 Compare August 9, 2023 09:27
@jonathanmos
jonathanmos merged commit f1c9ce2 into feature/base64-jmoskovich Aug 9, 2023
@jonathanmos
jonathanmos deleted the jmoskovich/replay-1890/bitmap-pool branch August 9, 2023 10:13
@xgouchet xgouchet added this to the 2.2.0 milestone Dec 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mariusc83 mariusc83 mariusc83 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

2.2.0

Development

Successfully merging this pull request may close these issues.

4 participants

@jonathanmos @codecov-commenter @mariusc83 @xgouchet