Reclaim used memory

[ghostwriter]

Q	A
Bugfix	yes

Description

• resolves Psr\SimpleCache\CacheInterface::clear() has no effect on hasAvailableSpace() #5
• supersedes/ closes Memory is not properly freed when removing data #7

[boesing]

Oh thats cool. Will have a look on this later on.
Just some questions here:

• can you add items to the memory Adapter after flush? You dropped that and thus it is not tested. what was wrong with the unit test in Memory is not properly freed when removing data #7?

• if the cache is empty and flush is used, it now returns false because nothing was freed, right? I'd add another sanity check to ensure it always returns true. I think we also need a test for that as it seems it is not yet tested?

• Should we call gc_mem_caches before and after the data is "cleared"? Maybe it frees up memory from some where else and thus returns true even tho nothing was actually freed?

[ghostwriter]

• can you add items to the memory Adapter after flush? You dropped that and thus it is not tested. what was wrong with the unit test in Memory is not properly freed when removing data #7?

Yes. It was an oversight on my part.

• if the cache is empty and flush is used, it now returns false because nothing was freed, right? I'd add another sanity check to ensure it always returns true. I think we also need a test for that as it seems it is not yet tested?

No. gc_mem_caches - Returns the number of bytes freed. (so, if no bytes were freed it returns 0.)

flush() will always return true, that behavior has not changed.

• Should we call gc_mem_caches before and after the data is "cleared"? Maybe it frees up memory from some where else and thus returns true even tho nothing was actually freed?

No. It has 0 noticeable difference in performance calling it before the data is "cleared".

[boesing]

No. It has 0 noticeable difference in performance calling it before the data is "cleared".

So what would you think about this? Would that work? Did not test that yet.

public function flush()
{
    if ($this->data === []) {
        return true;
    }
    
    gc_mem_caches();
    $this->data = [];

    return gc_mem_caches() > 0;
}

[boesing]

And, what was wrong with the unit test from #7? The unit test was originally provided by #5 and thus, thats an actual real-world example I guess.

When I apply your changes to my local copy, the test still fails. Any ideas?

[boesing]

#7 fails because $i is undefined $this->storage->addItem('item' . $i, sha1((string) mt_rand()));

In theory, yes. But running the tests - thats wrong. And as the OutOfSpaceException is thrown after $i is being defined, there is exactly no scenario where $i is not set. PHPUnit would fail with a notice error when that would be the case.

The test results are pretty clear:

1) LaminasTest\Cache\Storage\Adapter\MemoryTest::testReclaimMemory
Laminas\Cache\Exception\OutOfSpaceException: Memory usage exceeds limit (10485960).

/Users/max/git/laminas/laminas-cache-storage-adapter-memory/src/Memory.php:514
/Users/max/git/laminas/laminas-cache-storage-adapter-memory/vendor/laminas/laminas-cache/src/Storage/Adapter/AbstractAdapter.php:775
/Users/max/git/laminas/laminas-cache-storage-adapter-memory/test/unit/MemoryTest.php:64

[InvisibleSmiley]

Calling gc_mem_caches() also has no effect for me in my real-world testcase (which I cannot share unfortunately).

I'm still convinced that the actual issue lies somewhere in the laminas-cache base implementation (\Laminas\Cache\Storage\Adapter\AbstractAdapter::setItem), probably related to references getting attached to arrays inside objects in such a way that PHP's GC cannot resolve what is actual garbage that can be collected.

[boesing]

@InvisibleSmiley I removed any referencing from the code and ran the test. Same problem. Even tho I did not checked any LoC, I think its not related to circular referencing.

[boesing]

@ghostwriter can you please use the test case from #7 - if needed, put an $i = 0 outside the try {} catch () {} to avoid "undefined variable" issues.

I still end up having my unit tests fail when applying the code changes in here so either there is something different with the test cases in this PR or anything else is going on.

[ghostwriter]

Calling gc_mem_caches() also has no effect for me in my real-world testcase (which I cannot share unfortunately).

I'm still convinced that the actual issue lies somewhere in the laminas-cache base implementation (\Laminas\Cache\Storage\Adapter\AbstractAdapter::setItem), probably related to references getting attached to arrays inside objects in such a way that PHP's GC cannot resolve what is actual garbage that can be collected.

It could be laminas/laminas-cache#14 and or https://bugs.php.net/70098

[boesing]

Closing this for now as the approach of this PR might not result to what the initial author was expecting.
If there is still a need for this patch, please let me know.