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

Fixing preview caching #13609

Closed
6 of 11 tasks
oparoz opened this issue Jan 22, 2015 · 13 comments
Closed
6 of 11 tasks

Fixing preview caching #13609

oparoz opened this issue Jan 22, 2015 · 13 comments

Comments

@oparoz
Copy link
Contributor

oparoz commented Jan 22, 2015

This is a meta ticket used to reference all the various problems that need sorting so that preview generation and caching works quickly in all situations. The current solution is unbearably slow as soon as large images are involved.

Current situation

Asking for the first preview

This can be a Files thumbnail or a Gallery image per example, depending on how the picture was uploaded

The preview class collapses #13493

  • 9.0 if applying a simple local fix.
  • 9.0 if waiting on the move of the preview system to the AppFramework use appframework for preview system #13610. This would not solve all problems as some apps use the AppFramwork's PreviewManager internally

Large previews are returned by some preview providers #13607

Thumbnails of large files result in a time-out #13218

Save memory by streaming large previews straight from disk #13611

Asking for a new preview of different size

Caching seems to not be working #13516 #8196

Many PRs touching on thumbnail generation performance are affected by this, but the people most affected are the ones using large images, especially in formats which need to be converted by ImageMagick

How things will work

Once all those issues are fixed, we should have something that works like this

Asking for the first preview

  • Is size less than 50MB? Only process those
  • Do we have a max preview? If not, ask the provider for a 2048x2048 preview and it will return exactly that
  • Store that max preview, resize it to fulfil the request and send it back

Asking for a new preview of different size

  • Is a preview of that exact dimension stored already? Use that
  • If not, do we have a preview for this preview category? Use that
  • If not let's create a new preview for that category from the max preview and store it in the cache

@georgehrke Feel free to add any missing issues and PRs.
People who might be interested in this: @libasys @icewind1991

@georgehrke georgehrke added this to the 8.1-next milestone Jan 22, 2015
@georgehrke
Copy link
Contributor

Once enhancement we could add:

  • OC\Preview\Provider::getThumbnail will get a new parameter: a file handle

Instead of returning the OC_Image object, write the preview to the handle and just return true/false.

Any opinions?

@georgehrke
Copy link
Contributor

Another enhancement would be stream-based response types, that won't require us to load the cached preview into memory.
This was also requested here: #12988, not sure if there is a dedicated issue yet.

@georgehrke
Copy link
Contributor

@oparoz
Copy link
Contributor Author

oparoz commented Jan 22, 2015

Added StreamedResponse to the OP.
Regarding the handle, I'm not sure it would make a difference since the first thing the Image class is doing is loading the file to process it? What am I missing?

@georgehrke
Copy link
Contributor

I'm not sure it would make a difference since the first thing the Image class is doing is loading the file to process it?

  • There are preview generators, e.g. movies, that generate the png not using php.
  • Even if the preview generators use OC_Image, this way (combined with StreamedResponses) we would release the used memory earlier.

@oparoz
Copy link
Contributor Author

oparoz commented Jan 22, 2015

I guess if OC_Image doesn't need to load the data in memory to do its resizing, it would be a good thing to have, yes. We can already do that for some providers by using OC_Image::loadFromFileHandle no?

@oparoz
Copy link
Contributor Author

oparoz commented Jan 23, 2015

Previews generated by the bitmap preview class are now limited in their maximum dimensions: #13635

Affects:

  • PDF
  • Photoshop
  • Illustrator
  • Postscript
  • TIFF

@oparoz oparoz modified the milestones: 8.2-next, 8.1-current Mar 11, 2015
@oparoz
Copy link
Contributor Author

oparoz commented Mar 11, 2015

Most of it should be done by the time 8.1 launches if all the PR get merged, but some changes will have to wait for 8.2

@PVince81
Copy link
Contributor

@oparoz move to 9.0 as we're past feature freeze ?

@ghost ghost modified the milestones: 9.0-next, 8.2-current Sep 21, 2015
@ghost ghost modified the milestones: 9.1-next, 9.0-current Feb 23, 2016
@ghost ghost added the old-inactive label Feb 23, 2016
@PVince81 PVince81 modified the milestones: 9.1-current, 9.2-next Jun 14, 2016
@PVince81 PVince81 removed this from the 9.1-current milestone Jun 14, 2016
@PVince81 PVince81 modified the milestones: backlog, 10.0 Jan 27, 2017
@ownclouders
Copy link
Contributor

Hey, this issue has been closed because the label status/STALE is set and there were no updates for 7 days. Feel free to reopen this issue if you deem it appropriate.

(This is an automated comment from GitMate.io.)

@PVince81
Copy link
Contributor

@DeepDiver1975 since you were working on preview stuff.

@ownclouders
Copy link
Contributor

Hey, this issue has been closed because the label status/STALE is set and there were no updates for 7 days. Feel free to reopen this issue if you deem it appropriate.

(This is an automated comment from GitMate.io.)

@stale
Copy link

stale bot commented Sep 21, 2021

This issue has been automatically closed.

@stale stale bot closed this as completed Sep 21, 2021
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

6 participants