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

Possible disc cache key conflict - Wrong image returned #27

Closed
csobrinho opened this issue Oct 13, 2013 · 4 comments · Fixed by #29
Closed

Possible disc cache key conflict - Wrong image returned #27

csobrinho opened this issue Oct 13, 2013 · 4 comments · Fixed by #29
Labels

Comments

@csobrinho
Copy link
Contributor

I'm using Glide to load a lot of images and I'm getting sometimes a user profile picture instead of a country flag. Even after killing the process it is repetitive on the same page. I already saw this situation 2/3 times so my guess is that there should be a clash in the generator of the disc cache key.

@sjudd
Copy link
Collaborator

sjudd commented Oct 15, 2013

The two functions at fault are:

https://github.com/bumptech/glide/blob/master/library/src/com/bumptech/glide/resize/ImageManager.java#L627
https://github.com/bumptech/glide/blob/master/library/src/com/bumptech/glide/util/Util.java

The underlying reason for this bit of ugliness is that the disk cache implementation throws an exception if keys don't match the regex [a-z0-9_-]{1,64}. At some point someone has to be responsible for normalizing keys that don't match.

The Util.hash function should be improved and should probably return a 64 digit number rather than a 10 digit 32 bit integer which will help reduce collisions. Since the disk cache doesn't handle key collisions, we can't ever entirely solve this problem, but we should be able to make it very rare.

I just started a new job so I'm short on time, but if this seems reasonable to you I'll try and get something better up this week.

@csobrinho
Copy link
Contributor Author

Seems ok, I saw a discussion in SO about functions to calculate a better hash in a disc cache to avoid collisions but also improve the speed of the calculation. I think UUID was the winner but I'm not sure.

@csobrinho
Copy link
Contributor Author

Here it is:

Another possibility would be:

  • get the hashcode, convert it to 64, do a XOR|OR with the first/last X chars of the URL and convert to HEX
  • Base64 of the URL

@sjudd
Copy link
Collaborator

sjudd commented Oct 20, 2013

I saw your comment as I was finishing up something using sha-256. I'll take a look at the uuid questions. If it's a trade off between speed (uuid) and correctness (sha-256, 64 characters vs 32 for uuid), I'd probably rather use aggressive caching and be a bit slower the first time around.

I took a look at trace view and for uncached keys using sha-256, it took about 70ms to generate the keys for 26 or so different images. At 2-3ms per image, it's not a substantial performance hit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants