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

Performance issues with type-constraint checking #42

Closed
rsimoes opened this issue Oct 28, 2011 · 7 comments
Closed

Performance issues with type-constraint checking #42

rsimoes opened this issue Oct 28, 2011 · 7 comments

Comments

@rsimoes
Copy link

rsimoes commented Oct 28, 2011

Adapting Michael Schwern's benchmarking script mentioned in a blog post, I used it to test Method::Signatures type constraints. According to Method::Signature's documentation, the "run-time penalty if you do declare types should be very similar to using Mouse::Util::TypeConstraints (or Moose::Util::TypeConstraints) directly." I may have made a mistake in my own testing, but this appears to not be the case:

Testing Perl 5.014002, Method::Signatures 20111020, Moo 0.009011, Object::InsideOut 3.84, Mouse 0.97, Moose 2.0205
Benchmark: timing 6000000 iterations of Method::Signatures, Moo, Mouse, Object::InsideOut, manual...
Method::Signatures: 17 wallclock secs (17.10 usr +  0.00 sys = 17.10 CPU) @ 350877.19/s (n=6000000)
Moo:                10 wallclock secs (10.42 usr +  0.00 sys = 10.42 CPU) @ 575815.74/s (n=6000000)
Mouse:              2 wallclock secs ( 1.97 usr +  0.00 sys =  1.97 CPU) @ 3045685.28/s (n=6000000)
Object::InsideOut:  5 wallclock secs ( 5.56 usr +  0.00 sys =  5.56 CPU) @ 1079136.69/s (n=6000000)
manual:             11 wallclock secs (10.55 usr +  0.00 sys = 10.55 CPU) @ 568720.38/s (n=6000000)
Moose:              12 wallclock secs (12.26 usr +  0.00 sys = 12.26 CPU) @ 489396.41/s (n=6000000)

In order to get Method::Signatures to use Mouse, I commented out Moose and then reran the test with it included (incidentally, Method::Signature's time jumped to 30 seconds when it used Moose). My fork of the benchmark script can be found at https://gist.github.com/1321873 .

@schwern
Copy link
Contributor

schwern commented Oct 28, 2011

Method::Signatures is not intended to replace Mouse accessors. And your benchmark is not a fair fight. Method::Signature is doing more than the Mouse check. With METHOD_SIGNATURES_DEBUG set you can see that it's injecting...

my $self = shift;
Method::Signatures->type_check('Int', $_[0], 'arg') if (@_ > 0);
my $arg = $_[0]; Method::Signatures->too_many_args_error(1) if @_ > 1; 

Which is to say:

1) Bring in the invocant
2) Type check the first argument
3) Bring in the first argument
4) Make sure we haven't been given too many arguments

Doing that in the Mouse test looks like this:

{
    package Foo::Mouse;
    use Mouse;
    use Carp;
    has bar => ( is => 'rw', isa => 'Int' );

    sub test {
        my $self = shift;
        my $arg = shift;
        croak "Too many arguments" if @_ > 1;
        $self->bar($arg);
        return;
    }

    __PACKAGE__->meta->make_immutable;
}

Which is equivalent to this MS code.

{
    package Foo::Method::Signatures;
    use Mouse;
    use Method::Signatures;

    method test( Int $arg ) {
        $self->{bar} = $arg;
        return;
    }

    __PACKAGE__->meta->make_immutable;
}

I removed the set semantics since that's not what we're testing here. With that check, MS still turns out abysmally slow. 13 secs vs 4 seconds. So that ain't right. Lemme dive in and see what's gone wrong.

@schwern
Copy link
Contributor

schwern commented Oct 28, 2011

I profiled the type checking with:

# Run with NYTPROF=start=no perl -d:NYTProf <file>
{
    package Foo::Method::Signatures;
    use Mouse;
    use Method::Signatures;

    method test( Int $arg ) {
        $self->{bar} = $arg;
        return;
    }

    __PACKAGE__->meta->make_immutable;
}

my $obj = Foo::Method::Signatures->new;

DB::enable_profile();
$obj->test(23) for 1..10000;
DB::finish_profile();

and discovered that we're getting clobbered inside the type_check() by two lines:

                                                                    # find it if isn't cached
10000   21.3ms  10000   5.63ms  $mutc{cache}->{$type} ||= $class->_make_constraint($type);
    # spent 3.93ms making 9999 calls to Mouse::Meta::TypeConstraint::_identity, avg 393ns/call
    # spent 1.71ms making 1 call to Method::Signatures::_make_constraint

                                                                    # throw an error if the type check fails
    10000   29.8ms  10000   5.37ms  unless ($mutc{cache}->{$type}->check($value))
     # spent 5.37ms making 10000 calls to Mouse::Meta::TypeConstraint::check, avg 537ns/call

Those numbers are:

  • Calls to that line
  • Total time spent on that line
  • Statements executed on that line
  • Total time spent in subroutine calls on that line

What it tell me is those two lines, and not the subroutines they're calling, are sucking down 40ms. The time spent in type_check() is about 55ms. Ouch. So some micro-optimization is needed.

@schwern
Copy link
Contributor

schwern commented Oct 28, 2011

Oh, you know what it is? Mouse accessors and type checks are compiled XS code. Method::Signature's pure Perl type_check wrapper is competing with XS code. If you run with MOUSE_PUREPERL=1 you see the Mouse benchmark slow down quite a bit. We can't win this fight.

So the docs are correct, we're as fast as calling Mouse::Util::TypeConstraints directly, but we can't compete with the optimized accessor.

That said, type_check doesn't do much. It wouldn't be hard to write it in XS...

@barefootcoder
Copy link
Contributor

In addition to what Schwern pointed out, I would also note the following:

  • The fairest Moose comparison you could make is against MooseX::Params::Validate. This is the canonical (in the sense that it's what everybody on the Moose mailing list will tell you to use) way to validate parameters in Moose. My benchmarks comparing MS against MXPV show that we're significantly faster, but please feel free to try your own.
  • Your benchmarks make what I consider to be a very common mistake in evaluating parameter validation speed: that is, imagining that there's some method where all it ever needs to do is validate parameters. In the real world, this never actually happens. Parameter validation is typically a smaller percentage (and often a much smaller percentage) of the total work of the method. What I've done for my benchmarks is to use a loop to control the amount of extra work (beyond param validation) that's done, and base the number of times through the loop on a parameter passed to the method. That way, you can vary the value as much as you like and get a more realistic view on how much param validation speed impacts your real-world code.

Which is not to say that we can't still be faster, of course. :)

@schwern: Perhaps store the type check routine in a local var? Call _make_constraint every time and just have it return the cached version if it exists? I don't know ... at this point we're trading function calls for ors and scratchpad lookups for hash lookups ... how much extra can we squeeze out of it?

schwern added a commit that referenced this issue Oct 28, 2011
That extra subroutine call was slowing things down.
With XS Mouse they're about 40% faster.  With Moose they're about 20% faster.

If type_check() is overriden, we can't unroll it.

For #42
@schwern
Copy link
Contributor

schwern commented Oct 28, 2011

I inlined the contents of type_check(). Looks like this:

Method::Signatures->type_error('Int', $_[0], 'arg') if
    !($Method::Signatures::mutc{cache}{'Int'} ||= Method::Signatures->_make_constraint('Int'))->check($_[0]);

Eliminating the method call slashes off 40% of the runtime when using XS Mouse and 20% with Moose. I'm pretty happy with that. :-)

Made sure to check if type_check has been overridden first.

@barefootcoder
Copy link
Contributor

Eliminating the method call slashes off 40% of the runtime when using XS Mouse and 20% with Moose. I'm pretty happy with that. :-)

Sounds good. Shall we close this issue?

@schwern
Copy link
Contributor

schwern commented Oct 28, 2011

Yeah, I'm happy with the result. @rsimoes, if you have more to add just reply and we can reopen.

It can probably be made it faster once #34 is done. Then we know the type check mechanism at compile time and don't need the cache... well, that will require some changes to the way class/role/type is resolved. I'll worry about that later, I should be working on Test::Builder.

@schwern schwern closed this as completed Oct 28, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants