Use custom file storage solution to avoid Base64 encoding #62
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
weisers
reviewed
Nov 23, 2020
| mDiskHandler = new FileDiskHandler(context, mExecutorService); | ||
|
|
||
| // Clear out any data from old storage mechanism | ||
| mSharedPreferences.edit().clear().apply(); |
There was a problem hiding this comment.
Is there any plan to remove this at some point? Or are you just going to leave this here indefinitely?
There was a problem hiding this comment.
Yeah that's a good question. I'm not exactly sure the best way to go about getting rid of this. I would think that I'd have to keep it around for at least a few versions and then make a note when I finally remove it.
weisers
reviewed
Nov 23, 2020
| @@ -204,6 +216,7 @@ public void run() { | |||
| } | |||
| mPendingWriteTasks.add(runnable); | |||
| mExecutorService.execute(runnable); | |||
|
|
|||
There was a problem hiding this comment.
Feel free to remove this new extra line (unless for organizational purposes).
There was a problem hiding this comment.
I removed this line but also used this as an opportunity to replace mExecutorService.execute(runnable) with doInBackground(runnable).
weisers
reviewed
Nov 23, 2020
| * Stores the given {@code bytes} to disk and associates them with the given {@code key} for | ||
| * retrieval later. | ||
| * | ||
| * @param key The to associate with the saved data. |
weisers
reviewed
Nov 23, 2020
| private final Map<String, byte[]> mKeyByteMap = new ConcurrentHashMap<>(); | ||
|
|
||
| /** | ||
| * Determines whether or the {@link #waitForFilesToLoad()} call has completed in some way or |
byencho
force-pushed
the
improve-base64-encoding
branch
from
3f1492f
to
10ef326
Compare
weisers
approved these changes
Dec 4, 2020
byencho
force-pushed
the
improve-base64-encoding
branch
from
10ef326
to
c81c485
Compare
byencho
force-pushed
the
improve-base64-encoding
branch
from
c81c485
to
bd97329
Compare
|
Thanks @weisers |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR replaces the usage of
SharedPreferenceswith a custom file solution in order to avoid the need to Base64 encode the data before persisting it. This is to address OOM issues that can arise when callingBase64.encodeToString(#48).The original need for Base64 encoding was to get around the limitation that
SharedPreferencescould not be used to store a rawbyte[], so the closest thing we could get was to encode the data and store it as aString. With a custom file solution we can simply write the bytes directly and avoid the extra, memory-intensive encoding step. While I tried to keep my implementation (FileDiskHandler) as simple as possible, I did add a small feature that matches some of the behavior ofSharedPreferences: on startup, all of the file data will be read into memory on a background thread and the first call to retrieve data will block (up until a cutoff) until data has been read. This just helps ensure that all disk handling is front-loaded so that we don't block the main thread when navigating around and restoring data.