Bug #118665 for MooseX-Types: Memory Leak

Mon Nov 07 11:18:19 2016 keithbro256 [...] gmail.com - Ticket created

Subject:	Memory Leak - ArrayRef[Str]
Date:	Mon, 7 Nov 2016 16:18:07 +0000
To:	bug-MooseX-Types [...] rt.cpan.org
From:	Keith Broughton <keithbro256 [...] gmail.com>

Hi, Here is a test which fails due to a memory leak: use Test::Most; use Test::LeakTrace; use MooseX::Types::Moose qw(ArrayRef Str); use MooseX::Params::Validate; sub do_stuff { my ($self, $names) = validated_list(\@_, names => { isa => ArrayRef[Str] }, ); return 1; } no_leaks_ok { do_stuff(names => [ 'a', 'b' ]); }; done_testing; prove -l t/leak.t t/leak.t .. 1/? # Failed test 'leaks 6 <= 0' # at t/leak.t line 18. # '6' # <= # '0' Please let me know if I can provide any further information. Thanks Keith

Mon Nov 07 11:24:36 2016 DROLSKY [...] cpan.org - Correspondence added

On 2016-11-07 10:18:19, keithbro256@gmail.com wrote: Show quoted text

> Hi, > > Here is a test which fails due to a memory leak:

MXPV does some caching which could show up as a leak. So this may not be a real leak. If this _is_ a real leak, why do you think it's in MX::Types and not MXPV? Can you produce a leaking example using only MX::Types?

Mon Nov 07 11:24:37 2016 The RT System itself - Status changed from 'new' to 'open'

Mon Nov 07 12:11:45 2016 keithbro256 [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #118665] Memory Leak - ArrayRef[Str]
Date:	Mon, 7 Nov 2016 17:11:27 +0000
To:	bug-MooseX-Types [...] rt.cpan.org
From:	Keith Broughton <keithbro256 [...] gmail.com>

Hi Dave I think it's in MX::Types because when I replace ArrayRef[Str] with 'ArrayRef[Str]' (i.e. when I quote it) the test passes. I also have an application which is leaking memory with the MX::Types version but not the quoted one. Thanks Keith. On Mon, Nov 7, 2016 at 4:24 PM, Dave Rolsky via RT < bug-MooseX-Types@rt.cpan.org> wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=118665 > > > On 2016-11-07 10:18:19, keithbro256@gmail.com wrote:

> > Hi, > > > > Here is a test which fails due to a memory leak:

> > MXPV does some caching which could show up as a leak. So this may not be a > real leak. If this _is_ a real leak, why do you think it's in MX::Types and > not MXPV? Can you produce a leaking example using only MX::Types? > >

Mon Nov 07 12:20:59 2016 DROLSKY [...] cpan.org - Correspondence added

On 2016-11-07 11:11:45, keithbro256@gmail.com wrote: Show quoted text

> Hi Dave > > I think it's in MX::Types because when I replace ArrayRef[Str] with > 'ArrayRef[Str]' (i.e. when I quote it) the test passes. > > I also have an application which is leaking memory with the MX::Types > version but not the quoted one.

Thanks for the clarification! If I make a small change to the code like this: my $aos = ArrayRef[Str]; sub do_stuff { my ($self, $names) = validated_list(\@_, names => { isa => $aos }, ); return 1; } then the leak goes away. So I think the problem is that every time you hit this sub, it runs the 'ArrayRef[Str]' code, generating a _new_ type parameterized type constraint even though we've already made one that's functionally identical in this package. I think this is fairly fixable. As an side, you might want to take a look at using Params::ValidationCompiler instead of MXPV. That lets you build an optimized validation sub that inlined constraint checks. Based on testing I've done, this ends up being _way_ faster. If you're using MXPV a lot in your code base, replacing it with PVC globally can be a huge win. It's not quite a drop-in replacement, but it does provide nearly the same functionality.

Mon Nov 07 12:25:42 2016 DROLSKY [...] cpan.org - Correspondence added

On 2016-11-07 11:20:59, DROLSKY wrote: Show quoted text

> On 2016-11-07 11:11:45, keithbro256@gmail.com wrote:

> > Hi Dave > > > > I think it's in MX::Types because when I replace ArrayRef[Str] with > > 'ArrayRef[Str]' (i.e. when I quote it) the test passes. > > > > I also have an application which is leaking memory with the MX::Types > > version but not the quoted one.

> > Thanks for the clarification! > > If I make a small change to the code like this: > > my $aos = ArrayRef[Str]; > sub do_stuff { > my ($self, $names) = validated_list(\@_, > names => { isa => $aos }, > ); > > return 1; > } > > then the leak goes away. > > So I think the problem is that every time you hit this sub, it runs > the 'ArrayRef[Str]' code, generating a _new_ type parameterized type > constraint even though we've already made one that's functionally > identical in this package. I think this is fairly fixable. >

Actually, I just looked at the code and there's a comment about exactly this problem. It seems like it may not be fixable. All the more reason to try out PVC. It'll still make a new TC each time you write 'ArrayRef[Str]' but since this won't be executed each time a sub runs the leak will be fairly minor.

Mon Nov 07 12:35:33 2016 keithbro256 [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #118665] Memory Leak - ArrayRef[Str]
Date:	Mon, 7 Nov 2016 17:35:19 +0000
To:	bug-MooseX-Types [...] rt.cpan.org
From:	Keith Broughton <keithbro256 [...] gmail.com>

Thanks for your help, I'll give PVC a go! Keith. On Mon, Nov 7, 2016 at 5:25 PM, Dave Rolsky via RT < bug-MooseX-Types@rt.cpan.org> wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=118665 > > > On 2016-11-07 11:20:59, DROLSKY wrote:

> > On 2016-11-07 11:11:45, keithbro256@gmail.com wrote:

> > > Hi Dave > > > > > > I think it's in MX::Types because when I replace ArrayRef[Str] with > > > 'ArrayRef[Str]' (i.e. when I quote it) the test passes. > > > > > > I also have an application which is leaking memory with the MX::Types > > > version but not the quoted one.

> > > > Thanks for the clarification! > > > > If I make a small change to the code like this: > > > > my $aos = ArrayRef[Str]; > > sub do_stuff { > > my ($self, $names) = validated_list(\@_, > > names => { isa => $aos }, > > ); > > > > return 1; > > } > > > > then the leak goes away. > > > > So I think the problem is that every time you hit this sub, it runs > > the 'ArrayRef[Str]' code, generating a _new_ type parameterized type > > constraint even though we've already made one that's functionally > > identical in this package. I think this is fairly fixable. > >

> > Actually, I just looked at the code and there's a comment about exactly > this problem. It seems like it may not be fixable. All the more reason to > try out PVC. It'll still make a new TC each time you write 'ArrayRef[Str]' > but since this won't be executed each time a sub runs the leak will be > fairly minor. >

Bug #118665 for MooseX-Types: Memory Leak - ArrayRef[Str]