Skip to content
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

download + onlyRetrieveFromCache block #2428

Closed
hackerwgf opened this issue Sep 25, 2017 · 8 comments
Closed

download + onlyRetrieveFromCache block #2428

hackerwgf opened this issue Sep 25, 2017 · 8 comments
Milestone

Comments

@hackerwgf
Copy link

download image like this:

GlideApp.with(context)
.download(imageUrl)
.submit();

and load it onlyRetrieveFromCache at the same time:

new Thread(new Runnable() {
    @Override
    public void run() {
        try{
            GlideApp.with(context)
            .download(imageUrl)
            .onlyRetrieveFromCache(true)
            .submit()
            .get();
         }catch (Exception e){
             e.printStackTrace();
         }
    }
}).start();

then .get() will block

@sjudd
Copy link
Collaborator

sjudd commented Sep 25, 2017

.get() will block, yes. Is that what you were asking?

@sjudd sjudd added the question label Sep 25, 2017
@hackerwgf
Copy link
Author

So...why? I think .onlyRetrieveFromCache(true), it means that if no cache file here, then throw exception or just return null something else.
I apologize for my bad English

@sjudd
Copy link
Collaborator

sjudd commented Sep 26, 2017

You still need to do I/O to check the disk cache and decode the image if it's present.

@hackerwgf
Copy link
Author

I know it need time to do I/O stuff, maybe I did not express my meaning clearly.
In my situation, .get() will block by downloading the same imageUrl, not decoding or checking file.

A=.download(imageUrl).submit()
B=.download(imageUrl).onlyRetrieveFromCache(true).submit().get() in new thread
current situation:
start A >>>>>>downloading>>>>>>downloaded
start B >>>>>>blocking>>>>>>>>>got file

my expectation:
start A >>>>>>downloading>>>>>>downloaded
start B >>>>>>return error

@sjudd
Copy link
Collaborator

sjudd commented Sep 26, 2017

Ah, then start with: http://bumptech.github.io/glide/doc/debugging.html#unexpected-cache-misses. The Engine logs will tell you why there is/isn't a cache miss.

@hackerwgf
Copy link
Author

The log is "Added to existing load ...", that's why .get() blocked.
Maybe I should take the time to read source code for In-depth understanding.
.onlyRetrieveFromCache(true) makes me completely confused, especially the word ONLY.
So now look, is there a way to check cache file without blocking caused by existing load, or check a request status with url ?

@stale
Copy link

stale bot commented Nov 11, 2017

This issue has been automatically marked as stale because it has not had activity in the last seven days. It will be closed if no further activity occurs within the next seven days. Thank you for your contributions.

@Caij
Copy link

Caij commented Nov 12, 2017

load image from server, when the task is not completed, load same url request image only from cache, why second request need block wait first request complete(file down completed)? i think second request need return null.

the source code from:

EngineKey key = keyFactory.buildKey(model, signature, width, height, transformations,
resourceClass, transcodeClass, options);
EngineJob<?> current = jobs.get(key);
if (current != null) {
current.addCallback(cb);
if (Log.isLoggable(TAG, Log.VERBOSE)) {
logWithTimeAndKey("Added to existing load", startTime, key);
}
return new LoadStatus(cb, current);
}

@sjudd sjudd modified the milestone: 4.4 Nov 19, 2017
sjudd added a commit to sjudd/glide that referenced this issue Nov 23, 2017
This prevents cases where using onlyRetrieveFromCache might block a
request on an RPC if a request was already pending without
onlyRetrieveFromCache being set.

However, this change does now mean that it's possible a resource will be
loaded from the disk cache twice simultaneously because
onlyRetrieveFromCache requests are no longer deduped with normal
requests. We expect this case to be rare, and the consequences are an
efficiency issue rather than a correctness problem, so we're still
probably better off with this change.

Future work might include attempting to cancel and notify any normal
requests if an onlyRetrieveFromCache requests completes with the same
key will the normal request is running.

Fixes bumptech#2428.
sjudd added a commit to sjudd/glide that referenced this issue Nov 24, 2017
This prevents cases where using onlyRetrieveFromCache might block a
request on an RPC if a request was already pending without
onlyRetrieveFromCache being set.

However, this change does now mean that it's possible a resource will be
loaded from the disk cache twice simultaneously because
onlyRetrieveFromCache requests are no longer deduped with normal
requests. We expect this case to be rare, and the consequences are an
efficiency issue rather than a correctness problem, so we're still
probably better off with this change.

Future work might include attempting to cancel and notify any normal
requests if an onlyRetrieveFromCache requests completes with the same
key will the normal request is running.

Fixes bumptech#2428.
@sjudd sjudd closed this as completed in 108a062 Nov 24, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants