[PINCache] Set a default .byteLimit to reduce disk usage & startup time. #595
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
This default is fairly low - only 20MB - but for most apps with images in the size range of 10-50KB, this is still 400-1000 images. Once some optimizations land to PINCache, we'll match the PINCache default of 50MB to ensure the default better serves users with larger objects in the cache. Apps should preferably set their own byteLimit to an optimal value. @garrettmoon - one interesting question for us is the best place to set .byteLimit as an app. Digging into the ASPINRemoteImageDownloader and doing this type cast is a bit complicated, so a passthrough API to get the PIN* objects directly might be worthwhile.
|
🚫 CI failed with log |
|
@appleguy Yeah, that's a good idea, but I don't want to get all my Hacktoberfest PRs done in one day :P |
garrettmoon
reviewed
Sep 29, 2017
|
|
||
| // Set a default byteLimit. PINCache recently implemented a 50MB default (PR #201). | ||
| // Ensure that older versions of PINCache also have a byteLimit applied. | ||
| PINDiskCache *diskCache = [ASDynamicCast(cache, PINCache) diskCache]; |
There was a problem hiding this comment.
I think you need to add a <PINCache/PINCache.h> include at the top.
garrettmoon
approved these changes
Sep 29, 2017
|
Thanks! Looking into the right way to either reference, or avoid referencing PINCache as a symbol. |
|
🚫 CI failed with log |
appleguy
force-pushed
the
CacheDefault
branch
from
b63fc1e
to
596521e
Compare
|
@garrettmoon FYI, it looks like this test failure is spurious - it doesn't reproduce on my local machine, this patch isn't related, and it will probably pass upon retry. Had to do a fake commit change to poke the server. |
|
I had the same issue in a previous PR opened a couple of days ago. I had to manually retry the CI and the next time it finished successfully. |
bernieperez
pushed a commit
to AtomTickets/Texture
that referenced
this pull request
Apr 25, 2018
…me. (TextureGroup#595) * [PINCache] Set a default .byteLimit to reduce disk usage & startup time. This default is fairly low - only 20MB - but for most apps with images in the size range of 10-50KB, this is still 400-1000 images. Once some optimizations land to PINCache, we'll match the PINCache default of 50MB to ensure the default better serves users with larger objects in the cache. Apps should preferably set their own byteLimit to an optimal value. @garrettmoon - one interesting question for us is the best place to set .byteLimit as an app. Digging into the ASPINRemoteImageDownloader and doing this type cast is a bit complicated, so a passthrough API to get the PIN* objects directly might be worthwhile. * [PINCache] Declare necessary APIs to avoid a direct dependency.
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 default is fairly low - only 20MB - but for most apps with images
in the size range of 10-50KB, this is still 400-1000 images.
Once some optimizations land to PINCache, we'll match the PINCache
default of 50MB to ensure the default better serves users with larger
objects in the cache.
Apps should preferably set their own byteLimit to an optimal value.
@garrettmoon - one interesting question for us is the best place to
set .byteLimit as an app. Digging into the ASPINRemoteImageDownloader
and doing this type cast is a bit complicated, so a passthrough API
to get the PIN* objects directly might be worthwhile.