Re^4: given-when construct unexpected bahavior wit arrays

Oh, don't be silly. Last testcase. Ha! :-)

You're missing a whole slew of tests. And, to make matters worse, you're using timethis instead of cmpthese, which makes it so much harder to compare. So, first I'm going to provide, not necessarily the last test case, but the most recent (at the time of this writing) :-) And then I will comment on it, and then provide the output.

#!/usr/bin/env perl5.16.0

use 5.16.0;
use warnings;
use Benchmark qw/cmpthese/;
use List::MoreUtils qw/any/;

my @array = (0..10_000, 'abcd');
my %hash  = map { $_ => 1 } @array;
my @tests = (
             [ 'a' ],
             [ 'ab' ],
             [ 'abc' ],
             [ 0, 1 ],
             [ 1, 1 ],
             [ 10, 1 ],
             [ 100, 1 ],
             [ 1000, 1 ],
             [ 10000, 1 ],
             [ 'abcd', 1 ],
            );

sub run_tests {
    my $funcname = shift;
    my $func = __PACKAGE__->can($funcname);
    my $warnings;
    local $SIG{__WARN__} = sub { $warnings++ };

    for my $t (@tests)
    {
        $warnings = 0;
        my $rc = $func->($t->[0]);
        die "Invalid return ($funcname, $t->[0])\n" unless !$t->[1] ==
+ !$rc;
        warn "Had warnings ($funcname, $t->[0])\n" if $warnings;
    }
}

sub grep_in_array {
    my $element = shift;
    grep { $element eq $_ } @array and return 1;
    return;
}

sub brx_match {
    my $element = shift;
    $element eq $_ && return 1 for @array;
    return;
}

sub smartish_match {
    my $element = shift;
    given("_$element") {
        when ([map "_$_", @array]) { 1 }
        default { 0 }
    }
}

sub smartish_match_2 {
    my $element = shift;
    given ($element) {
        when ([map "$_", @array]) { 1 }
        default { 0 }
    }
}

sub smarthash {
    my $element = shift;
    given ($element) {
        when (\%hash) { 1 }
        default { 0 }
    }
}

sub smarthash_slow {
    my $element = shift;
    my %hash  = map { $_ => 1 } @array;
    given ($element) {
        when (\%hash) { 1 }
        default { 0 }
    }
}

sub any_match {
    my $element = shift;
    any { $element eq $_ } @array;
}

cmpthese -1, {
    grep => sub { run_tests 'grep_in_array' },
    brx  => sub { run_tests 'brx_match' },
    sm1  => sub { run_tests 'smartish_match' },
    sm2  => sub { run_tests 'smartish_match_2' },
    sh1  => sub { run_tests 'smarthash' },
    sh2  => sub { run_tests 'smarthash_slow' },
    any  => sub { run_tests 'any_match' },
};
[download]

Why 5.16? Because it's the latest. I don't think anything there needs anything more recent than 5.10. So a few things. First off, I'm combining all of the checks into a single run. This basically means we're taking average timings instead of best/worst case. I've also embedded testing the return values here just to make sure we don't end up with a function that is super-fast but wrong. I also capture warnings - since we're trying to do this without provoking warnings, again, testing it helps. This will all slow down each test, but all tests should get the same constant slowdown so the rankings should be the same even if the numbers aren't quite right.

I've moved the array to a global so we also aren't impacted by copying the array around. Again, it's constant, so it's just noise. But this is probably bigger noise than the above :-)

So, some additional tests. grep_in_array is your old one. brx_match is brx's suggestion (pretty good one). sm1 is your is_in_array while sm2 is a slight improvement on it (get rid of the underscore, it doesn't help). sh1/sh2 are the same as sm1/sm2 except using a hash base, with sh1 assuming that we're doing many matches against the array (thus the overhead of creating the hash can be ignored) and sh2 assuming we're doing one/few matches (thus the overhead of creating the hash is important). And any is just using List::MoreUtils' XS-based function. It's basically the same as brx' suggestion, but implemented in XS (aka C) instead. Oh, and it's already implemented and isn't subject to cut&paste errors or anything of the like.

Okay, now for the output:

        Rate     sh2     sm1     sm2    grep     brx     any     sh1
sh2   18.3/s      --    -30%    -33%    -87%    -92%    -93%   -100%
sm1   26.3/s     44%      --     -4%    -81%    -88%    -90%   -100%
sm2   27.4/s     49%      4%      --    -80%    -88%    -89%   -100%
grep   138/s    653%    424%    404%      --    -40%    -47%   -100%
brx    228/s   1145%    768%    735%     65%      --    -12%   -100%
any    258/s   1309%    882%    844%     87%     13%      --   -100%
sh1  52512/s 286329% 199445% 191840%  37948%  22899%  20227%      --
[download]

It should be of no surprise that sh1 completely blew the rest out of the water. The interesting bits are the 4% boost I got from eliminating the unneeded underscore from your original attempt, how much overhead creating the hash has (how slow sh2 is), how much better brx's suggestion is over plain grep (though that's not entirely average - on successful finds, we're weighted toward the first half of the array over the second half), and how much benefit there is (13%) to the XS version of brx's suggestion in List::MoreUtils::any.

Moral of the story: use hashes for lookups if you're doing repeated lookups. And if you're not, use CPAN modules.

Oh, and maybe I'll try opening the bug report for perl to fix this warning. It's still bugging me. :-)

Comment on Re^4: given-when construct unexpected bahavior wit arrays Select or Download Code