Psr\SimpleCache\CacheInterface::clear() has no effect on hasAvailableSpace()

[InvisibleSmiley]

Bug Report

The way the Memory adapter is currently implemented (using only memory_get_usage(true) as source of information), calling Psr\SimpleCache\CacheInterface::clear() has no effect on hasAvailableSpace().

Hence once a CacheException with previous OutOfSpaceException has been thrown, even calling clear() does not reclaim space as far as the Memory adapter is concerned. At this point the cache is permanently unusable because PHP won't ever reclaim allocated memory, only fetch more by 2 MiB increments.

Q	A
Version(s)	1.0.1

If this can't be fixed in some way, please at least document the limitation.

[weeman1337]

This test code reproduces the problem for me:

MemoryTest.php:

    public function testReclaimMemory()
    {
        // additional 200 bytes for up to 5 SHA1 items
        $this->_options->setMemoryLimit(memory_get_usage() + 200);

        try {
            for ($i = 0; $i <= 50; $i++) {
                $this->_storage->addItem('item' . $i, sha1(rand()));
            }

            self::fail('filling the cache with test data to reach the memory limit failed');
        } catch (OutOfSpaceException $ignore) {
        }

        $this->_storage->flush();
        $this->_storage->addItem('item' . $i, sha1(rand()));
    }

[weeman1337]

I have tried fixing this using memory_get_usage() and even gc_collect_cycles(). Unfurtunately that doesn't help. There are still references around somewhere.

After some research I have ended up here: https://github.com/laminas/laminas-cache/blob/2.10.x/src/Storage/Adapter/AbstractAdapter.php#L758

A quick experiment not using an ArrayObject to transport the event data seems to help. I am afraid that cannot be fixed easily.

[InvisibleSmiley]

So you say it's actually a laminas-cache issue?
Might be a duplicate of laminas/laminas-cache#14 then, depending on whether/how that will ever be fixed.
I guess for the time being I'll need to consider switching away from laminas-cache entirely then, and check which of the other PSR-16 implementations provide a memory-based adapter, and maybe even a size limit option (which is another, unrelated requirement I have).

BTW I thought that PHP might never free allocated memory as reported by memory_get_usage(true), but I found that's not the case (see below). Which supports the thought that the memory_get_usage(true) check is actually not the real issue.

<?php
function pm() { echo (memory_get_usage(true) / 1048576) . PHP_EOL; }

pm();

$a = [];
for ($i=0; $i<100_000; ++$i) {
    $o = new stdClass();
    $o->x = $i;
    $a[] = $o;
    if ($i % 10_000 === 0) {
        pm();
    }
}
pm();

unset($a);
pm();

gc_mem_caches();
pm();

prints:

2
2
6
10
14
22
26
30
36
40
44
48
42
6

[boesing]

Thanks for reporting this and thanks @weeman1337 for the failing test.
I've added it in #7 in order to have a starting point.

I've tested few things which did not work. Lets see if we can find the issue.

[boesing]

@InvisibleSmiley @weeman1337 might review #11?
Looks promising even tho I think it could need a field test in your project?

[boesing]

I guess the same error appears when only clearing some namespaces in case multiple namespaces are used?

[boesing]

Is this still a thing or did you switched to a non-laminas-cache-based PSR-16 library meanwhile?
If this bug is still a thing, please let me know and I'll reopen this.

[InvisibleSmiley]

Is this still a thing or did you switched to a non-laminas-cache-based PSR-16 library meanwhile? If this bug is still a thing, please let me know and I'll reopen this.

We still use this (or are at least trying to).

[InvisibleSmiley]

As discussed in laminas/laminas-cache#14 I tried laminas/laminas-cache 4.0.x together with an adapted version of this memory adapter to see whether the signature changes (esp. those reducing the arguments passed by reference) improved the situation. The answer is no.

Next I tried to replace ArrayObject with array but that didn't help either.

Then I started looking for memory profilers and gave https://github.com/arnaud-lb/php-memory-profiler a go.
It showed that memory was consumed not only by the test subjects itself but also PHPUnit (obviously) and the Composer autoloader (not so obvious, but somewhat understandable).

At first I thought maybe PHPUnit is the culprit so I rewrote the test to run standalone. Didn't help either.

Then I replaced the Composer autoloader by hardcoded "require" statements (which obviously need to be put in the correct order, i.e. interfaces before interface implementations etc.). This did the trick, i.e. the "real" PHP memory usage finally dropped after flushing the cache.

I don't have the time to dig deeper or try to investigate possible "fixes" to the Composer autoloader so this is where it ends for me.

Unless someone else is willing to look into it, I guess the only alternative would be to change all occurrences of memory_get_usage(true) to memory_get_usage(), i.e. only look at the used memory pages instead of the allocated ("real") ones. That value seems to drop after flushing the cache. Don't know what the broader consequences of such a change would be, but maybe it could be made configurable, defaulting to the current behavior.

For the sake of completeness, here's the final version of my test script.
Just comment the memprof stuff if you don't have the extension installed locally.
Also you may need to comment line 94 which not only references a class that seems to have a generated name (which might be different for your installation) but also requires the $loader property to be public (which needs to be done manually).

<?php

declare(strict_types=1);

use Laminas\Cache\Exception\OutOfSpaceException;
use Laminas\Cache\Storage\Adapter\Memory;
use Laminas\Cache\Storage\Adapter\MemoryOptions;
use Webmozart\Assert\Assert;

final class MemoryTest
{
    private const MEM_STEPPING = 2 * 1024 * 1024; // 2 MB

    private const LOOP_LIMIT = 10_000;

    public function testReclaimMemory(): void
    {
        //$loader = require_once __DIR__. '/vendor/autoload.php';
        require_once 'vendor/webmozart/assert/src/Mixin.php';
        require_once 'vendor/webmozart/assert/src/Assert.php';
        require_once 'vendor/laminas/laminas-cache/src/Storage/StorageInterface.php';
        require_once 'vendor/laminas/laminas-eventmanager/src/EventInterface.php';
        require_once 'vendor/laminas/laminas-eventmanager/src/Event.php';
        require_once 'vendor/laminas/laminas-eventmanager/src/SharedEventsCapableInterface.php';
        require_once 'vendor/laminas/laminas-eventmanager/src/EventManagerInterface.php';
        require_once 'vendor/laminas/laminas-eventmanager/src/EventManager.php';
        require_once 'vendor/laminas/laminas-eventmanager/src/EventsCapableInterface.php';
        require_once 'vendor/laminas/laminas-eventmanager/src/ResponseCollection.php';
        require_once 'vendor/laminas/laminas-cache/src/Storage/PluginCapableInterface.php';
        require_once 'vendor/laminas/laminas-cache/src/Storage/PluginAwareInterface.php';
        require_once 'vendor/laminas/laminas-cache/src/Storage/AvailableSpaceCapableInterface.php';
        require_once 'vendor/laminas/laminas-cache/src/Storage/ClearByPrefixInterface.php';
        require_once 'vendor/laminas/laminas-cache/src/Storage/ClearByNamespaceInterface.php';
        require_once 'vendor/laminas/laminas-cache/src/Storage/ClearExpiredInterface.php';
        require_once 'vendor/laminas/laminas-cache/src/Storage/FlushableInterface.php';
        require_once 'vendor/laminas/laminas-cache/src/Storage/IterableInterface.php';
        require_once 'vendor/laminas/laminas-cache/src/Storage/TaggableInterface.php';
        require_once 'vendor/laminas/laminas-cache/src/Storage/TotalSpaceCapableInterface.php';
        require_once 'vendor/laminas/laminas-cache/src/Storage/Event.php';
        require_once 'vendor/laminas/laminas-cache/src/Storage/PostEvent.php';
        require_once 'vendor/laminas/laminas-cache/src/Storage/ExceptionEvent.php';
        require_once 'vendor/laminas/laminas-cache/src/Exception/ExceptionInterface.php';
        require_once 'vendor/laminas/laminas-cache/src/Exception/OutOfSpaceException.php';
        require_once 'vendor/laminas/laminas-stdlib/src/ParameterObjectInterface.php';
        require_once 'vendor/laminas/laminas-stdlib/src/AbstractOptions.php';
        require_once 'vendor/laminas/laminas-cache/src/Storage/Adapter/AdapterOptions.php';
        require_once 'vendor/laminas/laminas-cache-storage-adapter-memory/src/MemoryOptions.php';
        require_once 'vendor/laminas/laminas-cache/src/Storage/Adapter/AbstractAdapter.php';
        require_once 'vendor/laminas/laminas-cache-storage-adapter-memory/src/Memory.php';

        // Autoload Assert (used by AbstractAdapter::addItem) to get its memory footprint addition out of the way
        Assert::boolean(true);

        $options = new MemoryOptions();
        $storage = new Memory($options);

        // AbstractAdapter::addItem triggers autoloading of Assert the first time it is called.
        // Call it once here to trigger that initial autoloading and only adjust the memory limit afterward.
        $storage->addItem('init', true);
        $storage->flush();

        $startMem = self::getMem();
        printf('%d real mem at start' . PHP_EOL, $startMem);

        // Write waste until all allocated but currently unused pages are filled
        $bin = [];
        $waste = 'The quick brown fox jumps over the lazy dog.';
        while (self::getMem() === $startMem) {
            $bin[] = $waste;
        }

        printf('%d real mem with unused pages filled' . PHP_EOL, self::getMem());

        $options->setMemoryLimit(self::getMem() + self::MEM_STEPPING);
        printf('%d = new real mem limit' . PHP_EOL, $options->getMemoryLimit());

        try {
            for ($i = 0; $i < self::LOOP_LIMIT; $i++) {
                $storage->addItem('item' . $i, $waste);
            }

            throw new Exception('Filling cache with test data to reach memory limit failed, got to ' . self::getMem());
        } catch (OutOfSpaceException $ignore) {
        }

        printf('%d real mem after exception' . PHP_EOL, self::getMem());
        printf('%d mem (used pages only) after exception' . PHP_EOL, memory_get_usage());

        $storage->flush();
        unset($bin);
        if (isset($loader)) {
            $loader->unregister();
            unset($loader);
            ComposerAutoloaderInit52b4f69827d264f7b84d362d8e816c99::$loader = null;
        }
        gc_collect_cycles();
        gc_mem_caches();

        printf('%d mem (used pages only) after flush' . PHP_EOL, memory_get_usage());
        printf('%d real mem after flush' . PHP_EOL, self::getMem());

        memprof_dump_callgrind(fopen("callgrind.out", "w"));

        $dump = memprof_dump_array();
        file_put_contents('array.out', print_r($dump, true));

        $storage->addItem('item' . $i, sha1((string) mt_rand()));

        assert(self::getMem() < $options->getMemoryLimit());
    }

    private static function getMem(): int
    {
        // Real memory usage (same as in Memory methods getAvailableSpace and hasAvailableSpace)
        return memory_get_usage(true);
    }
}

$test = new MemoryTest();
$test->testReclaimMemory();

[boesing]

Regarding the memory_limit, I've created an RFC to remove the memory_limit in favor of max_items.
I'd love to get some feedback in #57

Thats kinda the same approach as in symfony/cache and thus I guess its well accepted out there and could be an alternative approach to have cache keys limited rather than working with memory_limit which seems flawed anyway.