Port to Moo + Types::Standard

[ilmari]

Any::Moose is deprecated in favour of Moo, which integrates much better with both Moose and Mouse.
Use Types::Standard for the type constraints, and clean up the Response API a bit.

[petdance]

How does Moo compare with Any::Moose as far as memory usage? Execution speed?

[ilmari]

Here's a quick memory comparison:

$ perl -Ilib -e 'system ps => cu => 0+$$'
USER       PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
ilmari   26165  0.0  0.0  21504  1564 pts/55   S+   10:55   0:00 perl
$ git co master
Switched to branch 'master'
$ perl -Ilib -MWebService::Solr{,::Field,::Document,::Query,::Response} -e 'system ps => cu => 0+$$'
USER       PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
ilmari   26217  0.0  0.0  52184 10364 pts/55   S+   10:55   0:00 perl
$ git co mooification 
Switched to branch 'mooification'
$ perl -Ilib -MWebService::Solr{,::Field,::Document,::Query,::Response} -e 'system ps => cu => 0+$$'
USER       PID %CPU %MEM    VSZ   RSS TTY      STAT START   TIME COMMAND
ilmari   26235  0.0  0.0  60424 14024 pts/55   S+   10:55   0:00 perl

The Mouse (via Any::Moose) version adds 8.6MiB RSS on top of perl itself, while the Moo version adds 12.2MiB, so an extra 3.6MiB. The objects themselves are the same blessed hashes as always, so there should be no extra runtime memory overhead.

For performance comparison, I used http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=gitmo/Moo.git;a=blob;f=benchmark/object_factory, which showed Moo and Mouse to be within a few percent of each other, except for attributes with lazy defaults, where the XS version of Mouse is about twice as fast as Moo (2k/s vs 1k/s), because Class::XSAccessor can't accelerate the getter. This kind of attribute is only used in the Response class, and I don't foresee them being called frequently enough for this to make a difference.

[sjohnston]

If this were fixed to merge cleanly, would it be accepted?

[petdance]

Is Types::Standard another external module? Or is it part of Moo?

[sjohnston]

Types::Standard is part of Type::Tiny, another external module. The issue is that Moo does not include a basic type system so Type::Tiny was created to provide a type system compatible with Moo, Moose and Mouse.

[petdance]

I'm not real excited about the idea of adding another dependency, and adding 3.6MB of RAM usage, and slowing down some accessors. It seems to me the only reason to make the change is to make it be more like The Way Things Are Done These Days. Or am I missing something?

[sjohnston]

Any::Moose is officially deprecated and doesn't really provide adequate
compatibility with Moose anyway. If you are happy with Mouse and want to
keep minimal dependencies, then it is probably best to just use Mouse
directly.

On 04/01/2015 12:17 PM, Andy Lester wrote:

I'm not real excited about the idea of adding another dependency, and
adding 3.6MB of RAM usage, and slowing down some accessors. It seems to
me the only reason to make the change is to make it be more like The Way
Things Are Done These Days. Or am I missing something?

—
Reply to this email directly or view it on GitHub
#16 (comment).

[ilmari]

However, using Mouse directly will break things for people who use Moose to extend Webservice::Solr, like my employer does. Moo maintains compatibility with both Moose and Mouse.

[sjohnston]

Have you tried extending a Mouse class with Moose? It should be possible though there may be limitations I am not familiar with.

https://metacpan.org/source/GFUJI/Mouse-2.4.1/t/810_with_moose/500_moose_extends_mouse.t

[ilmari]

@sjohnston That test has a big fat [TODO] a Moose class cannot extends a Mouse class at the top, and this is one of the very reasons Moo was written in the first place.

[shadowcat-mst]

@sjohnston no, it isn't possible. You can create a Moo class (or role) in between Mouse and Moose and Moo's conversion code will kick in, but that's rather complicated. Basically if you want Moose compatibility and safe code, Moo was actually designed for that and as such is the only thing that actually works, sorry.

@petdance Any::Moose is broken by design - see http://shadow.cat/blog/matt-s-trout/moo-versus-any-moose/ for how it will cause random runtime failures for users

[lukec]

Hello - my logs are filling with this same deprecated warning.

If this pull request won't be accepted due to @petdance's concerns, what is the alternative?

[petdance]

I'll see if I can put out a dev version tonight, @lukec. My only real concern at this point is that switching to Moo will have bad effects for existing users, but @ilmari says "Moo maintains compatibility with both Moose and Mouse" and so that sounds good to me.

[lukec]

Thanks @petdance !!

[petdance]

Thanks, I've applied this at 3883d5b

[petdance]

Dev release 0.24_01 has been uploaded to PAUSE. Please test and let me know all is well so I can make a non-dev release.

[lukec]

Andy this release appears to work well for me. Thank you very much for spending time on this!

…

On Fri, Dec 9, 2016 at 9:14 PM, Andy Lester ***@***.***> wrote: Dev release 0.24_01 has been uploaded to PAUSE. Please test and let me know all is well so I can make a non-dev release. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#16 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAJ8EQVfVhLhwNeMm0UyMQ5o5BZqR-kks5rGjVRgaJpZM4BB2De> .

-- Luke Closs - Director of Operations ReCollect.net <https://recollect.net> | (360) 219-9778 Learn more <https://recollect.net> about how we've helped municipalities with their civic technology. This e-mail message contains proprietary information, and is intended solely for the use of the Individual or entity to whom it is addressed.

[petdance]

@ilmari Any comments?

[ilmari]

@petdance Thanks for merging this! I tested it with our Moose app at work when I did the port, but I'll re-test as soon as I can (possibly today).

[petdance]

Thanks, @ilmari. When you tell me you're happy with it, I'll make a 1.00 release.

[ilmari]

@petdance It passes our testsuite, which subclasses WebService::Solr to add JSON support, but we only use it for updates, not searching.

[lukec]

I tested it out for searching on our test system - without a subclass though - and it worked great.

…

On Wed, Dec 14, 2016 at 4:27 AM, Dagfinn Ilmari Mannsåker < ***@***.***> wrote: @petdance <https://github.com/petdance> It passes our testsuite, which subclasses WebService::Solr to add JSON support, but we only use it for updates, not searching. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#16 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAJ8N6KEYPlaypMn8JaocDhVtvIglsIks5rH-ClgaJpZM4BB2De> .

-- Luke Closs - Director of Operations ReCollect.net <https://recollect.net> | (360) 219-9778 Learn more <https://recollect.net> about how we've helped municipalities with their civic technology. This e-mail message contains proprietary information, and is intended solely for the use of the Individual or entity to whom it is addressed.

[lukec]

@petdance i have another datapoint to consider.

I have noticed the following warning from about $response->docs() being read-only. I suspect that in the dependency change, the docs field is going from implicit rw to ro.

The field is not declared ro/rw:

https://github.com/petdance/webservice-solr/blob/dev/lib/WebService/Solr/Response.pm#L24

But then a few lines below we use the setter method: https://github.com/petdance/webservice-solr/blob/dev/lib/WebService/Solr/Response.pm#L29

So in summary, I think docs should explicitly be listed as is => 'rw'.

[petdance]

@lukec I have moved that to its own ticket at #31

[lukec]

@petdance hello, checking in to see if we can move this patch forward. I'd love to see this on cpan if possible.

[petdance]

Oh my, it's been a month since that beta. Sorry about that. I'll upload a new one tonight.

[petdance]

I've just released 0.40 to CPAN. Thanks for the nudge.