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

Caching the generated images #25

Open
Mitch528 opened this issue Dec 4, 2017 · 8 comments
Open

Caching the generated images #25

Mitch528 opened this issue Dec 4, 2017 · 8 comments
Assignees

Comments

@Mitch528
Copy link
Collaborator

Mitch528 commented Dec 4, 2017

It would probably be a good idea to keep the images that are generated for a certain period of time so we're not generating a new image on every request. The image is also being overridden on each request and this can cause exceptions when the file is locked if there are concurrent requests.

Also, the file is named the same regardless of what the subtext is, which can cause conflicts if we were to cache the images. I think to resolve the naming conflicts, we can append a md5 hash of the subtext to the end of the filename.

@Kimmax
Copy link
Collaborator

Kimmax commented Dec 4, 2017

You're absolutely right
We als should cache the devRant avatars to take some load of the devRant avatar servers (I think they render them on every request)
Should also speed things up a lot.
About the filename, I'm not sure. We could also create a subfolder for each user and save using a guid.
But either way - I'm fine with that.

@Mitch528
Copy link
Collaborator Author

Mitch528 commented Dec 4, 2017

The problem with using a guid is that we wouldn't be able to find out which file is associated with the specified subtext.

Not really sure why that line would cause a memory leak though.

@Kimmax
Copy link
Collaborator

Kimmax commented Dec 4, 2017

Leak is taken care off, internale issue in the image library.
I agree with the GUID association. However I think we should stick to one saved / cached banner per user for now..
We can add more variations later, but let's keep it simple for the basics

@Mitch528
Copy link
Collaborator Author

Mitch528 commented Dec 4, 2017

Alright. If we're just going to do it per-user then caching may not make sense for the moment though. Otherwise, the subtext would not be able to be changed once it's been cached.

@Kimmax
Copy link
Collaborator

Kimmax commented Dec 4, 2017

Oh.. you're absolutely right.
Well let's skip the caching for users part for now and look into the avatar caching next
After that we should implement some sort of settings class to pass into the generator for the various options to come

@Kimmax
Copy link
Collaborator

Kimmax commented Dec 4, 2017

Avatar caching is in place. Keeping this issue on low prio for future banner caching

@cozyplanes
Copy link
Member

Isn't banner cached already? It seems the banners are saved in /generated.

@Kimmax
Copy link
Collaborator

Kimmax commented Dec 16, 2017

the least created one, yes

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

No branches or pull requests

3 participants