WIP -- work around php-ffi memory leaks

[jcupitt]

Due to a bug (I think?) in php-ffi we leaked c. 100 bytes for every call to getPspec(). This could really add up in long running programs.

Thanks @levmv!

See #167

[jcupitt]

Test program:

#!/usr/bin/env php
<?php

require __DIR__ . '/vendor/autoload.php';
use Jcupitt\Vips;

function vips($filename) {
    $image = Vips\Image::newFromFile($filename);
}

Vips\Config::cacheSetMax(0);

echo "iteration, growth (bytes)\n";
$prev = 0;
for ($i = 0; $i < 10000; $i++) {
    vips($argv[1]);

    if ($i % 100 == 0) {
        gc_collect_cycles();
        $now = memory_get_usage();
        $use = $now - $prev;
        $prev = $now;
        echo "$i, $use\n";
    }
}

With this PR I now see:

$ ./soak2.php ~/pics/k2.jpg
iteration, growth (bytes)
0, 1066192
100, 0
200, 0
...
1100, 0
9800, 0
9900, 0
$

So 10,000 iterations and no memory growth.

With master I see:

$ ./soak2.php ~/pics/k2.jpg
iteration, growth (bytes)
0, 1069248
100, 28928
200, 27392
300, 35584
400, 19200
...
9800, 19200
9900, 19200
$

A fairly steady 200 bytes per call.

[jcupitt]

Any testing very welcome!

[jcupitt]

If anyone is curious, the leak was caused by the line:

    return $pspec[0];

ie. dereferencing a managed array of pointers to unmanaged structs. php-ffi seems to make an unmanaged copy of the struct and the pointer in this case.

This new code:

https://github.com/libvips/php-vips/blob/revise-leaks/src/VipsObject.php#L109-L120

Does the dereference by hand (probably in a dumb way) and so fixes the leak.

[levmv]

Any testing very welcome!

A rather complex long-running script is working fine now! No memory issues so far in any use case.

[jcupitt]

This still leaks:

#!/usr/bin/env php
<?php

require __DIR__ . '/vendor/autoload.php';
use Jcupitt\Vips;

#Vips\Config::setLogger(new Vips\DebugLogger());

Vips\Config::cacheSetMax(0);

if (count($argv) != 4) {
    echo("usage: ./watermark-text.php input output \"some text\"\n");
    exit(1);
}

$input_filename = $argv[1];
$output_filename = $argv[2];
$message = $argv[3];

function watermark($image, $message)
{
    $text = Vips\Image::text($message, [
      'width' => $image->width,
      'dpi' => 600,
      'rgba' => true
    ]);

    // scale the alpha down to make it semi-transparent, and rotate by 45
    // degrees
    $text = $text
        ->multiply([1, 1, 1, 0.3])
        ->rotate(45);

    // replicate and crop to match the size of the image
    $text = $text
        ->replicate(1 + $image->width / $text->width,
            1 + $image->height / $text->height)
        ->crop(0, 0, $image->width, $image->height);

    // and overlay on the image
    return $image->composite($text, "over");
}

echo "iteration, growth (bytes)\n";
$prev = 0;
for ($i = 0; $i < 2000; $i++) {
    $image = Vips\Image::newFromFile($input_filename, [
      'access' => 'sequential',
    ]);

    // no leak
    //$image = $image->invert()

    // leaks 192 bytes per loop
    $image = $image->add(1);

    // leaks 288 bytes per loop
    //$image = watermark($image, $message);

    $image->writeToFile($output_filename);

    if ($i % 10 == 0) {
        gc_collect_cycles();
        $now = memory_get_usage();
        $use = $now - $prev;
        $prev = $now;
        echo "$i, $use\n";
    }
}

Looks like a problem with constant handling, I'll have a look.

[levmv]

Hm... Looks like my complex code was accidentally not so complex (It has most of this operations, but depends on input and config). I'm sorry.

[jcupitt]

Heh no problem, we nailed one leak, I'm sure there's just one more.

[jcupitt]

I found another one -- php-ffi leaks 96 bytes every time you call arrayType. I switched to using strings instead and this is now leak-free:

#!/usr/bin/env php
<?php

require __DIR__ . '/vendor/autoload.php';
use Jcupitt\Vips;

// Vips\Config::setLogger(new Vips\DebugLogger());
// Vips\Config::cacheSetMax(0);

if (count($argv) != 4) {
    echo("usage: ./watermark-text.php input output \"some text\"\n");
    exit(1);
}

$input_filename = $argv[1];
$output_filename = $argv[2];
$message = $argv[3];

function watermark($image, $message)
{
    $text = Vips\Image::text($message, [
      'width' => min(300, $image->width),
      'height' => min(300, $image->height),
      'align' => 'centre',
      'rgba' => true
    ]);

    // scale the alpha down to make it semi-transparent, and rotate by 45
    // degrees
    $text = $text
        ->multiply([1, 1, 1, 0.3])
        ->rotate(45);

    // replicate and crop to match the size of the image
    $text = $text
        ->replicate(1 + $image->width / $text->width,
            1 + $image->height / $text->height)
        ->crop(0, 0, $image->width, $image->height);

    // and overlay on the image
    return $image->composite($text, "over");
}

echo "iteration, growth (bytes)\n";
$prev = 0;
for ($i = 0; $i < 1000; $i++) {
    $image = Vips\Image::newFromFile($input_filename, [
      'access' => 'sequential',
    ]);

    $image = watermark($image, $message);

    $image->writeToFile($output_filename);

    if ($i % 10 == 0) {
        gc_collect_cycles();
        $now = memory_get_usage();
        $use = $now - $prev;
        $prev = $now;
        echo "$i, $use\n";
    }
}

I see:

iteration, growth (bytes)
0, 1115408
10, 0
20, 0
...
980, 0
990, 0
$

Any more testing would be great!

[levmv]

My naive search also led me to GValue/arrayType - so, a little proud of myself :) But while I tried to understand it a bit (FFI/c/memory things - are hard!) - i see you already found it and fix it, so cool!

Testing it now, so far so good.

[levmv]

Script with newFromBuffer, newFromFile, bandjoin, add, rot180, resize, thumbnail_buffer, embed, black, cast, insert, composite, extract_band, gravity, gaussblur, icc_import, colourspace, jpegsave_buffer - works fine. Same visible output, no leaks.

[jcupitt]

OK, let's merge and put out a new version with these fixes. Thanks!

[jcupitt]

OK, tagged master as v2.1.1