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 consumption and closures #40

Closed
plicease opened this issue Feb 25, 2015 · 6 comments
Closed

Memory consumption and closures #40

plicease opened this issue Feb 25, 2015 · 6 comments
Labels
🐣Enhancement Things that make it work better

Comments

@plicease
Copy link
Member

Reopening in the right place from here:

PerlFFI/FFI-Platypus-Lang-CPP#8

This test cases consumes memory, until it runs out:

use FFI::Platypus;
use FFI::TinyCC;

sub c {
}

my $tcc = FFI::TinyCC->new;

$tcc->compile_string(<<'EOF');
void c_with_arg(void (*f)(void))
{
    f();
}
EOF

my $ffi = FFI::Platypus->new;
$ffi->type('()->void', 'FType');

my $c = $ffi->closure(\&c);

my $a = $tcc->get_symbol('c_with_arg');

while(1) {
    $ffi->function($a => ['FType'] => 'int')->call($c);
}

If you cast the closure to an opaque before the loop then you are good:

use FFI::Platypus;
use FFI::TinyCC;

sub c {
}

my $tcc = FFI::TinyCC->new;

$tcc->compile_string(<<'EOF');
void c_with_arg(void (*f)(void))
{
    f();
}
EOF

my $ffi = FFI::Platypus->new;
$ffi->type('()->void', 'FType');

my $c = $ffi->closure(\&c);
my $c_ptr = $ffi->cast('()->void' => 'opaque', $c);

my $a = $tcc->get_symbol('c_with_arg');

while(1) {
    $ffi->function($a => ['opaque'] => 'int')->call($c_ptr);
}

Possible fixes:

  1. Instead of pushing the closure data onto a list, cache it in a hash where the key is the signature. Could the signature be the pointer to the FFI::Platypus::Type? Probably good enough since the type should be reused in the FFI::Platypus instance.
  2. Document as in the CAVEATS section, with the above alternate suggestion. This is reasonable if the use case is not common.
@pipcet
Copy link
Contributor

pipcet commented Feb 25, 2015

Thank you! I'd come up with a similar "solution", except mine used a tiny TinyCC function to convert the closure to a pointer:

$tcc->compile_string(q{
    void *
    identity(void *arg)
    {
    return arg;
    }
});

my $identity = $tcc->get_symbol('identity');
my $function_pointer = $ffi->function($identity => ['ErrorReporter'] => 'opaque')->call($closure);

(I'm also passing $function_pointer to an Inline::CPP-generated function, which Just Works).

Using ->cast() would be much cleaner (and actually avoid the need for TinyCC entirely in my case), though.

I think it would also be nice to be able to pass opaque pointers instead of closures to a function without changing the signature:

$ffi->function($a => ['()->void'] => 'void')->call($c_ptr);

I'll have a look at the code to see how hard that would be to implement.

Thank you again!

@plicease
Copy link
Member Author

I look forward to the PR :)

@pipcet
Copy link
Contributor

pipcet commented Feb 25, 2015

It's at #41 (I've added documentation and a test case to the original pull request, but in essence it's a one-liner). Let me know if anything else needs changing.

@plicease
Copy link
Member Author

The doco and the test cases are essential, including it saves me from doing it. #41 should be included in 0.30 when it is released.

@pipcet
Copy link
Contributor

pipcet commented Feb 25, 2015

Okay, I've gone ahead and implemented cached closures based on the FFI type as key. It seemed straightforward enough and passes the existing tests, but I've run into a new problem: FFI::Platypus::Closure actually modifies (blesses) its argument, so calling it twice with the same argument causes trouble:

use FFI::Platypus;

sub c {
    return 7;
}

my $ffi = FFI::Platypus->new;
my $c = \&c;

$ffi->closure($c);
$ffi->closure($c);

The obvious solution is to make a FFI::Platypus::Closure an ordinary blessed hashref rather than a blessed coderef, and overload calling if it's really needed. Working on that now.

@plicease
Copy link
Member Author

This should be fixed in 0.30 on its way to CPAN.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐣Enhancement Things that make it work better
Development

No branches or pull requests

2 participants