Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory is not properly freed when removing data #7

Closed
wants to merge 2 commits into from

Conversation

boesing
Copy link
Member

@boesing boesing commented Feb 27, 2021

Q A
Bugfix yes/no
BC Break yes/no

Description

As of #5, a test was provided to ensure that memory is properly freed after calling flush.
Thats indeed not the case.

I've played around a bit aswell as in #5 was an assumption that it might be due to the ArrayObject by-reference pass of the key/value in AbstractAdapter of laminas-cache. I cannot confirm this as I've removed the ArrayObject and its still impossible to get memory freed.

I have lacks in knowledge of PHP memory handling and thus just want to provide the failing test here.
Help from the @laminas/technical-steering-committee and all others around would be highly appreciated.

Closing #5

@boesing

This comment has been minimized.

@boesing boesing changed the base branch from 1.0.x to 1.1.x April 28, 2021 17:28
Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
Thanks to Michael Weimann

Signed-off-by: Maximilian Bösing <2189546+boesing@users.noreply.github.com>
@Xerkus
Copy link
Member

Xerkus commented May 10, 2021

on a cursory glance there are 2 issues:

  • looks like a bug - memory_get_usage(true) with true provides allocated memory to the process. It is not the amount that is actually used within the allocated space.
  • php does not immediately collect garbage. use gc_collect_cycles() in catch clause in test to immediately force GC run

@boesing
Copy link
Member Author

boesing commented May 10, 2021

  • looks like a bug - memory_get_usage(true) with true provides allocated memory to the process. It is not the amount that is actually used within the allocated space.

Exactly. And that is the reason why we are allocating + 200 extra bytes which can be used.

  • php does not immediately collect garbage. use gc_collect_cycles() in catch clause in test to immediately force GC run

I've already tried that - no difference.

@Xerkus
Copy link
Member

Xerkus commented May 10, 2021

Then again, the problem is that system allocated memory limit might be more important than the amount of actually utilized memory for some cases. Especially if app is run in environment where exceeding allocated memory limit will get app killed.

I have no idea how php manages memory, like does it always allocate new memory and releases/reuses only once each specific page is fully used? Does it reuse partially utilized pages?

@Xerkus
Copy link
Member

Xerkus commented May 10, 2021

I would say if you bounce off your more or less hard limit for the memory adapter you do not want to go and allocate more memory.

that makes me inclined towards wontfix and documentation note.

Also, do test with 4kb increments - the average page size

@ghostwriter ghostwriter mentioned this pull request May 11, 2021
@boesing
Copy link
Member Author

boesing commented May 31, 2021

Closing in favor of #11

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Help Wanted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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