Moo conversion #140

itcharlie · 2015-05-23T05:51:12Z

Mouse to Moo conversion changes for issue #94. Thanks to Yanick for the orientation and help.

Moo is now included in one of the test files.

some more work on Moo

All tests are passing

tokuhirom · 2015-05-26T07:48:05Z

There are some failing tests...

itcharlie · 2015-05-26T23:50:06Z

We are good now. Thanks @tokuhirom and @yanick

syohex · 2015-08-04T14:56:35Z

Benchmark(Moo vs Mouse)

I benchmark Moo version and Mouse version. Benchmark scripts are in benchmark directory.

Environment

Intel Core i7-2620M CPU @ 2.70GHz
Memory 8GB
Ubuntu 15.04
Perl 5.22.0
Mouse 2.4.3
Moo 2.000002

cascade.pl

Moo

        Rate MTEx   TX
MTEx  3258/s   -- -90%
TX   31508/s 867%   --

Mouse

        Rate MTEx   TX
MTEx  3229/s   -- -90%
TX   32288/s 900%   --

data_section.pl

Moo

            Rate  file/1  file/2 vpath/1 vpath/2
file/1  104052/s      --    -39%    -41%    -43%
file/2  170836/s     64%      --     -3%     -7%
vpath/1 175812/s     69%      3%      --     -4%
vpath/2 183794/s     77%      8%      5%      --

Mouse

            Rate  file/1  file/2 vpath/1 vpath/2
file/1  106455/s      --    -42%    -42%    -44%
file/2  183991/s     73%      --     -0%     -4%
vpath/1 184721/s     74%      0%      --     -3%
vpath/2 190934/s     79%      4%      3%      --

demo-mt.pl

Moo

In this benchmark, Xslate is about 5.8 times faster than Text::MicroTemplate.

Mouse

In this benchmark, Xslate is about 6.0 times faster than Text::MicroTemplate.

demo-tt.pl

Moo

In this benchmark, Xslate is about 91.5 times faster than Template-Tookit.

Mouse

In this benchmark, Xslate is about 90.9 times faster than Template-Tookit.

expr.pl

Moo

          Rate     mt  s///g xslate
mt      3622/s     --    -9%   -86%
s///g   3999/s    10%     --   -84%
xslate 25121/s   594%   528%     --

Mouse

          Rate     mt  s///g xslate
mt      3556/s     --    -6%   -87%
s///g   3794/s     7%     --   -86%
xslate 26613/s   648%   602%     --

expr_eq.el

Moo

          Rate     mt  s///g xslate
mt      3490/s     --   -11%   -85%
s///g   3929/s    13%     --   -83%
xslate 22755/s   552%   479%     --

Mouse

          Rate     mt  s///g xslate
mt      3479/s     --   -13%   -88%
s///g   3999/s    15%     --   -86%
xslate 28444/s   718%   611%     --

foo.pl

Moo

         Rate     mt     ht xslate
mt     1581/s     --   -44%   -83%
ht     2844/s    80%     --   -69%
xslate 9050/s   472%   218%     --p

Mouse

         Rate     mt     ht xslate
mt     1555/s     --   -45%   -83%
ht     2844/s    83%     --   -69%
xslate 9135/s   488%   221%     --

include.pl

Moo

          Rate     TT     MT     HT Xslate
TT       153/s     --   -93%   -95%   -99%
MT      2283/s  1395%     --   -32%   -82%
HT      3350/s  2093%    47%     --   -74%
Xslate 12922/s  8361%   466%   286%     --

Mouse

          Rate     TT     MT     HT Xslate
TT       155/s     --   -93%   -95%   -99%
MT      2349/s  1419%     --   -29%   -82%
HT      3319/s  2046%    41%     --   -75%
Xslate 13274/s  8484%   465%   300%     --

interpolate.pl

Moo

              Rate        TMT      s///g     xslate    sprintf xslate/raw
TMT         2791/s         --       -34%       -75%       -92%       -94%
s///g       4225/s        51%         --       -62%       -88%       -90%
xslate     11061/s       296%       162%         --       -67%       -75%
sprintf    33810/s      1112%       700%       206%         --       -22%
xslate/raw 43530/s      1460%       930%       294%        29%         --

Mouse

              Rate        TMT      s///g     xslate    sprintf xslate/raw
TMT         2791/s         --       -35%       -76%       -92%       -94%
s///g       4266/s        53%         --       -64%       -88%       -91%
xslate     11821/s       324%       177%         --       -65%       -74%
sprintf    34133/s      1123%       700%       189%         --       -25%
xslate/raw 45510/s      1531%       967%       285%        33%         --

json.pl

Moo

          Rate xslate   json
xslate 18335/s     --    -1%
json   18442/s     1%     --

Mouse

          Rate   json xslate
json   17568/s     --    -1%
xslate 17772/s     1%     --

x-poor-env.pl

Moo

          Rate     TT Xslate     HT     MT
TT     43.1/s     --    -5%   -54%   -79%
Xslate 45.4/s     5%     --   -52%   -77%
HT     94.2/s   119%   108%     --   -53%
MT      201/s   366%   343%   113%     --

Mouse

         Rate     TT Xslate     HT     MT
TT     43.1/s     --    -2%   -57%   -78%
Xslate 44.2/s     3%     --   -56%   -78%
HT      101/s   134%   128%     --   -49%
MT      199/s   361%   350%    97%     --

x-rich-env.pl

Moo

          Rate     TT     MT    HTP Xslate
TT      72.4/s     --   -83%   -97%   -99%
MT       435/s   502%     --   -81%   -96%
HTP     2283/s  3054%   424%     --   -78%
Xslate 10288/s 14114%  2263%   351%     --

Mouse

          Rate     TT     MT    HTP Xslate
TT      72.4/s     --   -83%   -97%   -99%
MT       415/s   473%     --   -82%   -96%
HTP     2311/s  3093%   457%     --   -79%
Xslate 11164/s 15325%  2591%   383%     --

Finally

Almost case, Mouse version got good result than Moo version.

zdm · 2015-08-05T10:55:53Z

Any plans to release this code?

GeJ · 2015-08-05T21:29:00Z

As syohex showed, Mouse performs better in most benchmarks.
Hopefully, the maintainers will stick with Mouse.

karenetheridge · 2015-12-04T18:32:01Z

@itcharlie would it be possible for you to rebase this PR against the latest master (and possibly squash some of those merge commits down)? I think some of the performance differences can be improved. It would be a shame for this work not to be merged, as Moo is preferred over Mouse these days.

karenetheridge · 2015-12-04T18:56:45Z

lib/Text/Xslate/Runner.pm

@@ -212,12 +213,14 @@ sub getopt_spec { @Spec }
 sub _build_getopt_spec {
    my($self) = @_;

+    $DB::single = 1;


This line should be removed.

yanick · 2015-12-04T19:08:19Z

@karenetheridge @itcharlie I can do the rebase, if you want. Shouldn't be too hard.

itcharlie · 2015-12-04T19:15:11Z

Thanks Yanick,

@karenetheridge I don't think I will be able to sit down and work on this
right away. If Yanick can do this right away I suggest he work on this
instead of me.

best,

Charlie

Charlie Gonzalez
(E) [email protected]
Github Profile https://github.com/itcharlie

On Fri, Dec 4, 2015 at 2:08 PM, Yanick Champoux [email protected]
wrote:

@karenetheridge https://github.com/karenetheridge @itcharlie
https://github.com/itcharlie I can do the rebase, if you want.
Shouldn't be too hard.

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

genehack · 2015-12-04T19:22:41Z

+1 for this changeset. Xslate is the only thing I routinely use that is dependent on Mouse and it would be extremely disappointing to see this work not accepted, especially given that the benchmark differences are pretty negligible.

karenetheridge · 2015-12-04T19:26:56Z

lib/Text/Xslate/Runner.pm

@@ -212,12 +213,14 @@ sub getopt_spec { @Spec }
 sub _build_getopt_spec {
    my($self) = @_;

+    $DB::single = 1;
+
    my @spec;
    foreach my $attr($self->meta->get_all_attributes) {


aha, this would be a contributor to the slowdown -- this Moo class is being upgraded to Moose to get the list of attributes. (thanks @shadowcat_mst)

yanick · 2015-12-04T19:31:14Z

Rebase to master at https://github.com/yanick/p5-Text-Xslate/tree/moo_conversion

I also spotted that the tests are still using Mouse. I'm going to take care of that in the next few hours

shadowcat-mst · 2015-12-04T19:46:22Z

Symbol.pm was a heavy user, solved via

sub clone {
    my $self = shift;
    return bless({ %$self },ref($self));
}

sub _dump_denote {
    my($self, $type, $parser) = @_;

    my $entity = exists $self->{$attr}
        ? $self->{$attr}
        : $parser->can('default_' . $type);

    require B;
    my $cvgv = B::svref_2object($entity)->GV;
    printf STDERR "%s: %s::%s (%s:%s)\n",
        $type,
        $cvgv->STASH->NAME, $cvgv->NAME,
        $self->id, $self->line,
    ;
}

I can only get a 1% benchmark difference because almost all the work in almost all the benchmarks is being done by ->render's XS code. Well, except for the poor-env test where Moo appears to be loading slightly faster but I'm not sure that's really a significant difference either.

The getopt stuff in Runner needs tweaking as well but that didn't show up at the top of my profile (MooX::Options would maybe solve this, or borrowing part of its implementation)

shadowcat-mst · 2015-12-04T19:51:17Z

If somebody can propose a benchmark that shows a more-than-1% slowdown in a way I can actually proile, I wouldn't mind poking this with a stick. The Symbol thing was basically the only thing I managed to get a handle on from the benchmark scripts. It's friday evening though, so I may just be being stupid.

yanick · 2015-12-04T20:29:30Z

Switched types to be 'Types::Standard' in my branch, which should also speed up the Moo version.

GeJ · 2015-12-04T23:54:22Z

-1 with this. Just because people can't stand to add one standalone module to their module list, they are willing to impose several new modules to others. @gfx already expressed that he was not willing to switch to Moo but would consider plain objects. Text::Xslate works fine with Mouse, and it doesn't need to be "fixed" with Moo.

Personally, I don't want to add Moo and its dependencies to my module set. Already did, and it cost me a week to revert because people knew better than me how I should handle warnings. Not anymore!

shadowcat-mst · 2015-12-25T20:22:20Z

@GeJ If you don't want to write warning clean code, Moo 2.0 in March no longer fatalises warnings by default. If you'd like to learn how, strictures 2 now only fatalises warnings we can be reasonably sure won't cause forward compatibility issues.

Also, if you have a pre-2.0 Moo you can easily do -

use Moo;
use warnings NONFATAL => 'all';

or use Moo::Lax or ...

Plenty of options other than reversion and they're all basically a single perl -pi -e away :)

GeJ · 2015-12-27T22:03:33Z

@shadowcat-mst I perfectly know that. That's the quick fix I implemented two years ago when I was asked to leave a Sunday family gathering by a pissed off boss who got harassed by an angry client who was asking why none of his orders were processed in a timely fashion.

When I went to the office on that day I realized that someone, somewhere thought that appending an undefined value to a string was such an offense to the programming Gods that it should warrant killing any piece of code that would attempt to do so. And that's my problem with Moo : I don't like such clear POLA violations and I can't trust people who commit them. And rather than doing a rollback, Moo users had to use a workaround for roughly a year.

Also, and aside any personal opinion I may have on Moo, the root of this PR is just for convenience. There is no technical merit in this. s/Mouse/Moo/g everywhere in the code is not enough, it is slower than the original. Looking back at the PR, it has been suggested to add even more extra modules to catch up, or worse : drop the Moo API and directly tap into the guts of the object to speed up things. Wow!

I will repeat myself : just because people don't want to add one standalone module, they are willing to impose their convoluted solution to others. Mouse works just fine. It may not have all the bells and whistles of Moose or Moo, but it is in my opinion the perfect example of a much simple solution that fits the needs of a given problem. If memory serves, when I removed every reference to Moo in my code, there were no Moo-only feature I used that couldn't be replaced by either a Mouse extension or half a dozen lines of code.

Finally, a careful reader might remember that a respected member of the Perl community qualified Mouse as "amazing code", adding "If you're going to use Mouse, use Mouse." So, can we please leave it like that and stop trying to fix a problem that doesn't exist.

Edit: added a few edits this and there to fix a few mistakes. Please forgive my possibly awful English as it is not my native language.

zdm · 2015-12-27T22:46:07Z

+1 for Moo. Because Moo is more widely used and benchmark shows, that is is not slower that Mouse (several percents are standard error).

shadowcat-mst · 2016-01-04T19:27:16Z

And that's my problem with Moo : I don't like such clear POLA violations and I can't trust people who commit them.

It was already clearly documented. I'm sorry you missed it.

And rather than doing a rollback, Moo users had to use a workaround for roughly a year.

The first ever release of Moo fatalized warnings. The delay was to try and minimise the damage to downstream users who -liked- them and regarded them as an important part of their code's integrity. Your "rollback" would've been their "sudden backcompat breakage", and that wasn't acceptable to us.

abraxxa · 2017-09-07T11:01:23Z

Will this get merged and released?

yanick · 2017-09-07T13:28:07Z

I turned my own copy of this branch as a PR, considering it's freshly rebased and contain a few extra tweaks (as per the comments above)

yanick and others added 15 commits May 2, 2015 15:13

s/Mouse/Moo/

c3fcebe

so it begins...

4478606

converting

9f06a6e

progressing

19e2fd5

done with Text::Xslate::Parser?

5694a08

continuing...

606ddd3

More moo changes..

e763567

Moo is now included in one of the test files.

Enum[]

77a6156

add the types

64c10f3

Moo don't do lazy_build

425c468

Merge pull request #1 from yanick/moo

743c4fd

some more work on Moo

more changes

c5ccd18

more tests passing

2ca4727

Need Moo > 2.00 for non-strictness

962f541

Merge pull request #2 from yanick/moo

3a94b36

All tests are passing

Update makefile and modules with Moo version

71d9f57

karenetheridge reviewed Dec 4, 2015
View reviewed changes

yanick mentioned this pull request Sep 7, 2017

Moo conversion #178

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Moo conversion #140

Moo conversion #140

itcharlie commented May 23, 2015

tokuhirom commented May 26, 2015

itcharlie commented May 26, 2015

syohex commented Aug 4, 2015

zdm commented Aug 5, 2015

GeJ commented Aug 5, 2015

karenetheridge commented Dec 4, 2015

karenetheridge Dec 4, 2015

yanick commented Dec 4, 2015

itcharlie commented Dec 4, 2015

genehack commented Dec 4, 2015

karenetheridge Dec 4, 2015

yanick commented Dec 4, 2015

shadowcat-mst commented Dec 4, 2015

shadowcat-mst commented Dec 4, 2015

yanick commented Dec 4, 2015

GeJ commented Dec 4, 2015

shadowcat-mst commented Dec 25, 2015

GeJ commented Dec 27, 2015

zdm commented Dec 27, 2015

shadowcat-mst commented Jan 4, 2016

abraxxa commented Sep 7, 2017

yanick commented Sep 7, 2017

Moo conversion #140

Are you sure you want to change the base?

Moo conversion #140

Conversation

itcharlie commented May 23, 2015

tokuhirom commented May 26, 2015

itcharlie commented May 26, 2015

syohex commented Aug 4, 2015

Benchmark(Moo vs Mouse)

Environment

cascade.pl

Moo

Mouse

data_section.pl

Moo

Mouse

demo-mt.pl

Moo

Mouse

demo-tt.pl

Moo

Mouse

expr.pl

Moo

Mouse

expr_eq.el

Moo

Mouse

foo.pl

Moo

Mouse

include.pl

Moo

Mouse

interpolate.pl

Moo

Mouse

json.pl

Moo

Mouse

x-poor-env.pl

Moo

Mouse

x-rich-env.pl

Moo

Mouse

Finally

zdm commented Aug 5, 2015

GeJ commented Aug 5, 2015

karenetheridge commented Dec 4, 2015

karenetheridge Dec 4, 2015

Choose a reason for hiding this comment

yanick commented Dec 4, 2015

itcharlie commented Dec 4, 2015

genehack commented Dec 4, 2015

karenetheridge Dec 4, 2015

Choose a reason for hiding this comment

yanick commented Dec 4, 2015

shadowcat-mst commented Dec 4, 2015

shadowcat-mst commented Dec 4, 2015

yanick commented Dec 4, 2015

GeJ commented Dec 4, 2015

shadowcat-mst commented Dec 25, 2015

GeJ commented Dec 27, 2015

zdm commented Dec 27, 2015

shadowcat-mst commented Jan 4, 2016

abraxxa commented Sep 7, 2017

yanick commented Sep 7, 2017