Skip to content
This repository has been archived by the owner on Nov 1, 2020. It is now read-only.

No need to construct the image in memory #265

Closed
wants to merge 1 commit into from
Closed

No need to construct the image in memory #265

wants to merge 1 commit into from

Conversation

rullzer
Copy link
Member

@rullzer rullzer commented May 5, 2017 •
edited by MorrisJobke
Loading

Not constructing the image in memory saves valuable resources

TODO:

  • Fix tests

@rullzer
No need to construct the image in memory
46dbc7c 
Not constructing the image in memory saves valuable resources

* Pass around File or ISimpleFile objects
* Use the FileDisplayResponse so we have all the fancy caching

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@rullzer rullzer added 2. developing Work in progress enhancement New feature or request performances Performances issues and optimisations labels May 5, 2017
@rullzer rullzer added this to the Nextcloud 12.0 milestone May 5, 2017
@rullzer rullzer requested a review from oparoz May 5, 2017 18:55
oparoz
oparoz suggested changes May 8, 2017
View reviewed changes
Copy link
Member

@oparoz oparoz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of the changes made on the PreviewController also need to be made on the PreviewApiController unfortunately as they behave slightly differently.

https://github.com/nextcloud/gallery/blob/46dbc7cc6107d8ee1556acf63cc12033b1e88982/lib/Controller/PreviewApiController.php

That's one reason a lot of the tests fail

@MorrisJobke
Copy link
Member

I guess also this needs to be moved to 13

@oparoz oparoz modified the milestones: Nextcloud 13, Nextcloud 12.0 May 19, 2017
@oparoz
Copy link
Member

oparoz commented May 19, 2017

Unfortunately, yes.

@MorrisJobke
Copy link
Member

Moved to 14.

@MorrisJobke MorrisJobke mentioned this pull request Aug 30, 2018
Merged
6 tasks
@MorrisJobke
Copy link
Member

Moved to 15

@MorrisJobke MorrisJobke mentioned this pull request Nov 6, 2018
Merged
29 tasks
@MorrisJobke MorrisJobke removed this from the Nextcloud 15 milestone Nov 8, 2018
@nickvergessen
Copy link
Member

The gallery app has been replaced by the beautiful new app:
Nextcloud Photos - 📸 Your memories under your control

Please checkout if your Pull request is still necessary there, and in case create it there or raise an issue for others to copy the change from here.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@oparoz oparoz oparoz requested changes

Assignees
No one assigned
Labels
2. developing Work in progress enhancement New feature or request performances Performances issues and optimisations
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Don't store images in memory
4 participants
@rullzer @MorrisJobke @oparoz @nickvergessen