Skip to content

Imagine\Gd\Image::thumbnail() memory usage #149

Closed
noginn opened this Issue Aug 22, 2012 · 31 comments

9 participants

@noginn
noginn commented Aug 22, 2012

When trying to create a thumbnail of a large image (3000x3000) the memory usage of my script exceeds 128MB.

The issue seems to be because a copy of the original image is created before resizing occurs.

I've managed to work around this by copying the thumbnail sizing logic into my own code and calling "resize" rather than "thumbnail" as it doesn't create a copy. I do not need the original image so this works for me.

It would be useful to be able to resize an image based on thumbnail modes. Is this something that you would be interested in including? I didn't want to do the work and create a pull request before checking.

Thanks

@avalanche123
Owner

The problem is that if we resize the original image before making a copy of it, further operations will be affected. You could use imagick/gmagick that wouldn't take up so much memory

@romainneutron
Collaborator

An alternative is to set up your environment with more memory :

<?php
ini_set('memory_limit', '1024M');

As defined in http://php.net/manual/en/ini.core.php, memory_limit is PHP_INI_ALL so it can be set up with such function.

see http://fr.php.net/manual/en/function.ini-set.php

@noginn
noginn commented Aug 23, 2012

The resize method does resize without making a copy though, so what I was asking for was essentially a way of resizing using thumbnail modes not changing the way thumbnail() works as I understand that would be a big change of behaviour.

One way I thought this could work was by moving the scaling logic from the Image classes into Imagine\Image\Box. So something along the lines of this would work:

<?php

$size = $image->getSize();
$thumbnailSize = $size->inset(new Box(60, 60));
$image->resize($thumbnailSize);

// ...

public function inset(BoxInterface $size)
{
    $width  = $size->getWidth();
    $height = $size->getHeight();

    $ratio = min(array(
        $width / $this->getWidth(),
        $height / $this->getHeight()
    ));

    return $this->scale($ratio);
}

What do you think about moving that logic? That way I wouldn't need to duplicate the logic just to resize an image in the same way as a inset thumbnail.

@romainneutron
Collaborator

I don't understand ; do you have a memory or logic issue ?

@noginn
noginn commented Aug 23, 2012

Memory issue, which could be helped by a change in logic.

For example, a large 3000x3000 image is uploaded and needs to be resized.

Using Imagine\Gd\Image::thumbnail() a new image resource is created with the same 3000x3000 dimensions, then resized. Creating the 3000x3000 image resource uses a lot of memory as expected.

Using Imagine\Gd\Image::resize() a new image resource is created with the resized dimensions (in my case 60x60), so the memory usage is a lot less.

I now understand why a changing Imagine\Gd\Image::thumbnail() to not create a copy wouldn't work, but it would be useful to have the inset thumbnail logic available so it is possible to resize without excessive memory usage.

I'm happy to extend Imagine\Image\Box myself but thought it might be useful to others. Unless there is another way to do the same thing that I am missing?

Thanks

@romainneutron
Collaborator

The best way is to propose a code change by doing a PR

@afoeder
afoeder commented Jan 30, 2013

I'm just fiddling with this issue at the moment; and I ask myself why it's not possible to refactor the thumbnail() method to something like

$thumbnail = clone($this)
$thumbnail->resize(...)

?

@noginn, you wrote

I now understand why a changing Imagine\Gd\Image::thumbnail() to not create a copy wouldn't work

but me didn't, why wouldn't that work? On resize(), as you already wrote, a new imageresource is created anyways, so the code behind this mustn't be changed.

@afoeder afoeder added a commit that referenced this issue Jan 30, 2013
@afoeder afoeder [WIP][BUGFIX] Improve memory consumption for thumbnails
When creating a thumbnail, no longer a copy of the initial,
probably very large image is created, but the resizing
happens directly on the initial image.

For convenience, two new `BoxInterface` methods have been
introduced, `inset()` and `outbound()`, which will provide
an according resizing of a `Box`. The `Image`'s `thumbnail`
method has been refactored to make use of these methods. Credits
go to @noginn for the idea and the code snippet.

Fixes #149
014d2d1
@afoeder
afoeder commented Jan 30, 2013

if you like you can fiddle with afoeder/Imagine@5766798 in order to see the results. In my case the consumption improved from 11.5 to 6.75 MB memory usage.

@avalanche123
Owner

would be great to wrap this up as a PR and move discussion there

@afoeder
afoeder commented Feb 5, 2013

Gladly; see here: #197

@rochadk
rochadk commented Sep 22, 2013

What's the status for this issue? It still seems like there's memory issues when using thumbnail

@romainneutron
Collaborator

I don't see any memory issue :

running this script

$start = memory_get_usage();

for ($test = 0; $test <= 2; $test++) {
    $imagine = new Imagine\Gd\Imagine();
    $imagineLoad = memory_get_usage();
    $image = $imagine->open('large-image-2400x3200.jpg');
    $imageLoad = memory_get_usage();

    echo "memory : $start (start) $imagineLoad (Imagine load) $imageLoad ($imageLoad)\n";

    for ($i = 1; $i <= 10; $i++) {
            $before = memory_get_usage();
            $image->thumbnail(new Imagine\Image\Box(300, 200))->save('thumbnailed-'.$i.'.jpg');
            $after = memory_get_usage();
            echo "memory : $before -> $after\n";
    }

    unset($image);
    $imageUnload = memory_get_usage();
    unset($imagine);
    $imagineUnload = memory_get_usage();

    echo "memory : $imagineUnload (Imagine unload) $imageUnload (image unload)\n\n";
}

produces this output :

memory : 328560 (start) 389848 (Imagine load) 41151736 (41151736)
memory : 41152392 -> 41217640
memory : 41217920 -> 41217920
memory : 41217920 -> 41217920
memory : 41217920 -> 41217920
memory : 41217920 -> 41217920
memory : 41217920 -> 41217920
memory : 41217920 -> 41217920
memory : 41217920 -> 41217920
memory : 41217920 -> 41217920
memory : 41217920 -> 41217920
memory : 782832 (Imagine unload) 787296 (image unload)

memory : 328560 (start) 787856 (Imagine load) 41218464 (41218464)
memory : 41218464 -> 41218464
memory : 41218464 -> 41218464
memory : 41218464 -> 41218464
memory : 41218464 -> 41218464
memory : 41218464 -> 41218464
memory : 41218464 -> 41218464
memory : 41218464 -> 41218464
memory : 41218464 -> 41218464
memory : 41218464 -> 41218464
memory : 41218464 -> 41218464
memory : 783112 (Imagine unload) 787856 (image unload)

memory : 328560 (start) 787856 (Imagine load) 41218464 (41218464)
memory : 41218464 -> 41218464
memory : 41218464 -> 41218464
memory : 41218464 -> 41218464
memory : 41218464 -> 41218464
memory : 41218464 -> 41218464
memory : 41218464 -> 41218464
memory : 41218464 -> 41218464
memory : 41218464 -> 41218464
memory : 41218464 -> 41218464
memory : 41218464 -> 41218464
memory : 783112 (Imagine unload) 787856 (image unload)

The ImageInterface::thumbnail method returns a copy of the image (for extracting multiple thumbnails from a single Image object).

@romainneutron
Collaborator

But I understand that INSET / OUTBOUND modes could be nice on a resize method or filter ; this should be the point of another issue (feature request).

I'm thinking of closing this issue. Please provide some code / PR if you think I shouldn't

@staabm
staabm commented Sep 22, 2013

I think they dont mean a memory leak, but a high peak usage (depending on the origin image size)

@romainneutron
Collaborator

Yes I do think so, but it's actually normal, dealing with large image needs memory, right ? :)

@staabm
staabm commented Sep 22, 2013

Sure thats right. In case the lib has a way to create a thumbnail without the need to copy the whole image at first, they could create thumbnails of even bigger images without lifting mem limit.
(In ither words the opeation itself would consume less memory)

@rochadk
rochadk commented Sep 25, 2013

I'm running this code:

$sizes = array(
        'thumb' => array(
            'size' => new \Imagine\Image\Box(50, 50),
            'mode' => ImageInterface::THUMBNAIL_OUTBOUND
        ),
        'list' => array(
            'size' => new \Imagine\Image\Box(100, 75),
            'mode' => ImageInterface::THUMBNAIL_INSET
        ),
        'preview' => array(
            'size' => new \Imagine\Image\Box(250, 188),
            'mode' => ImageInterface::THUMBNAIL_INSET
        )
    );

    $img = new Imagine();
    $img = $img->open($path . '/' . $newFilename);

    foreach ($sizes as $size => $info) {
        $img->thumbnail($info['size'], $info['mode'])->save($path . '/' . $size . '_' . $newFilename);
    }

And it produces an FatalErrorException: Error: Allowed memory size of 134217728 bytes exhausted (tried to allocate 31961089 bytes)

The image I'm trying to upload is 2448x3264px and 3.03MB.. could it be GD that uses to much memory?

I just tried using resize instead, and that works just fine

@romainneutron
Collaborator

You could try to monitor/log memory usage before / during Imagine processing to get what's wrong. It should be ok : as you don't assign the thmbnail to a variable, it should be garbage collected as soon as it's saved.

Btw, which version of PHP are you using ?

@wvdweij
wvdweij commented Sep 28, 2013

The issue is clear, there should be a way to create a thumbnail from an image without making a copy. I'm running into the same issues. Huge memory allocation when performing thumbnail creation on big images. The memory usage is doubled because of the copy it creates from the original src image.

I created a little workaround:

Ajdust the creation of the copy on line 323 in Imagine\Gd\Image.php

if ($copy === true)
{
$thumbnail = $this->copy();
}
else
{
$thumbnail = $this;
}

And adjust the thumbnail method on line 309:

public function thumbnail(BoxInterface $size, $mode = ImageInterface::THUMBNAIL_INSET, $copy = true)

We can just call the thumbnail function with:
$image->thumbnail(new Box($width, $height), ImageInterface::THUMBNAIL_OUTBOUND, false);

@romainneutron
Collaborator

Hello, you think this an issue, I'd rather consider it a feature.

creating multiple high quality thumbnail from a single high res is common usage and only possible with this implementation.

Use the resize method if memory is limited.

@wvdweij
wvdweij commented Sep 28, 2013

I understand, it's a feature. I mean, shouldn't there be a way to make it easy to use the thumbnail feature without the creation of copies? In my code example in the previous comment I added an extra parameter to the thumbnail method to allow or disallow the creation of copies.

@romainneutron
Collaborator

But I understand that INSET / OUTBOUND
modes could be nice on a resize method or
filter ; this should be the point of another
issue (feature request).

let's do it !

@wvdweij
wvdweij commented Sep 28, 2013

Yeah! Thx man! btw i'm loving your library keep up the good work!

@rochadk
rochadk commented Sep 28, 2013

I'm using php 5.3.

Sounds great with the INSET / OUTBOUND on resize, that would solve my issue :)

@noginn
noginn commented Sep 29, 2013

I'm happy for the issue to be closed. My original suggestion was to move the inset/outbound methods so they could be re-used by the resize method. This is what I did to solve the memory issue in my application but never got around to creating a PR.

@noginn noginn closed this Sep 29, 2013
@afoeder
afoeder commented Oct 10, 2013

what the heck is with my pull request, btw? Amazingly totally ignored, it seems...

@EgidioCaprino

Guys, I'm sorry for the spam but could you please answer this simple question?

#263

Thank you.

Egidio

@romainneutron
Collaborator

Guys, please do not consider this issue as a forum, please keep discussions in their appropriate threads.

@afoeder Your PR is tagged as "WIP" so I'm waiting for a final PR to review and comment.
@Aegidius I'll have a look at your question

@EgidioCaprino

@romainneutron thank you very much!

@afoeder
afoeder commented Oct 10, 2013

@romainneutron thanks for your care, it's WIP because of avalanche123 having said

would be great to wrap this up as a PR and move discussion there

but, no discussion moved there. It's old and stale anyways, just wanted to mention that nobody cared even when this thread was "exhumed".

@Petah
Petah commented Oct 8, 2014

How much memory is expected to be required to resize a 6000x4000 image?

https://github.com/Petah/imagine-memory

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.