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 not being released #30

Closed
thecrypticace opened this issue Aug 1, 2016 · 6 comments
Closed

Memory not being released #30

thecrypticace opened this issue Aug 1, 2016 · 6 comments
Labels

Comments

@thecrypticace
Copy link

thecrypticace commented Aug 1, 2016

It appears that allocated space is not being freed resulting in large memory usage. I believe entire object graphs are staying around when stored in sets.

I've tested these classes that all seem to have the problem:

  • Set
  • Map
  • Stack
  • Queue
  • Deque

Notes: Neither calling clear() on the set nor the use of gc_collect_cycles() frees any memory.

Example reproduction:

$original = memory_get_usage(true);

for ($i = 0; $i < 10000000; $i++) {
    new \Ds\Set(["A", "B"]);
}

echo memory_get_usage(true) - $original, " additional bytes allocated\n";

Output:

79691776 additional bytes allocated

Environment:

  • PHP DS compiled from master
  • PHP 7.0.9 64-bit
  • macOS 10.11.6 / Ubuntu 14.04

Edit: 10x iteration count + output changes to showcase larger memory footprint

@rtheunissen
Copy link
Member

It's useful to run this using a debug build, which shows that there is a leak, and that the allocation was made here: https://github.com/php-ds/extension/blob/master/src/ds/ds_set.c#L12

ds_set_t *set = ecalloc(1, sizeof(ds_set_t));

We're obviously not freeing sets properly, and we should also check for memory leaks in CI. I'll open an issue to discuss to best way to do that.

rtheunissen added a commit that referenced this issue Aug 3, 2016
@rtheunissen
Copy link
Member

Fixed in e937fb5

@thecrypticace
Copy link
Author

thecrypticace commented Aug 3, 2016

It's useful to run this using a debug build,

That's good to know. I might try setting one up on my end to test things.

Fixed in …

It appears that as soon as a union, intersection, xor, etc… are performed memory will still be leaked.

While C is not my strong suit I know my way around some of it. I might try my hand at and fixing some of these issues. I'm sure PRs fixing (or — attempting to) could be helpful?

@rtheunissen
Copy link
Member

While C is not my strong suit I know my way around some of it. I might try my hand at and fixing some of these issues. I'm sure PRs fixing (or — attempting to) could be helpful?

Your detective work has been incredibly helpful to the point where I get excited when I see you've created a new issue or PR.

PR's fixing would definitely be helpful though, would be cool to have someone else dig into the C as well. It's very satisfying to fix these.

It appears that as soon as a union, intersection, xor, etc… are performed memory will still be leaked.

How can you tell? 🤔

@thecrypticace
Copy link
Author

thecrypticace commented Aug 4, 2016

I boiled it down to a test case:

$original = memory_get_usage(true);

for ($i = 0; $i < 10e4; $i++) {
    $a = new \Ds\Set([]);
    $b = new \Ds\Set([]);
    $a->xor($b);
}

echo memory_get_usage(true) - $original, " additional bytes allocated\n";

Output:

60817408 additional bytes allocated

Aside: I've got another issue where php-ds freezes related to sets containing Hashable objects containing sets containing Hashable objects (… and then turtles?). I'll open another issue with the details.

@rtheunissen
Copy link
Member

rtheunissen commented Aug 4, 2016

Debug build output for a single loop shows:

[Thu Aug  4 17:05:39 2016]  Script:  '/path/to/leak-test.php'
/path/to/ds/src/ds/ds_set.c(12) :  Freeing 0x105202078 (8 bytes), script=/path/to/leak-test.php
/path/to/php/Zend/zend_alloc.c(2510) : Actual location (location was relayed)
[Thu Aug  4 17:05:39 2016]  Script:  '/path/to/leak-test.php'
/path/to/ds/src/ds/ds_htable.c(142) :  Freeing 0x105203CC0 (32 bytes), script=/path/to/leak-test.php
/path/to/php/Zend/zend_alloc.c(2510) : Actual location (location was relayed)
[Thu Aug  4 17:05:39 2016]  Script:  '/path/to/leak-test.php'
/path/to/ds/src/ds/ds_htable.c(37) :  Freeing 0x10525ABA0 (64 bytes), script=/path/to/leak-test.php
[Thu Aug  4 17:05:39 2016]  Script:  '/path/to/leak-test.php'
/path/to/ds/src/ds/ds_htable.c(27) :  Freeing 0x105268500 (512 bytes), script=/path/to/leak-test.php
/path/to/php/Zend/zend_alloc.c(2510) : Actual location (location was relayed)
=== Total 4 memory leaks detected ===

Which collapses down to:

ds_set.c(12)
ds_htable.c(142)
ds_htable.c(37)
ds_htable.c(27)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants