stevan / promises-perl

An implementation of Promises in Perl
31 stars 29 forks source link

Cherry pick performance idea from PR #61 #83

Closed ehuelsmann closed 5 years ago

ehuelsmann commented 5 years ago

There's a very nice performance gain to be had from PR #61, but that change contains other (disputed) changes. The performance gain (2 to 3 times more performance for AnyEvent/AE and more than 5 times performance increase for EV) to be had is too good to let it whither away in an unmerged PR.

Thanks thanks to @TvdW for the ideas!

ehuelsmann commented 5 years ago

The performance tests have been performed with variants of the test program pasted below. Should I submit a PR with the benchmarks for the backends that I have now? (Default, AnyEvent, AE and EV)

#!/usr/bin/perl

use v5.18;
use warnings;
use Benchmark qw/timethese/;
use Promises qw/collect deferred/, backend => [ 'EV' ];

print "Backend:  $Promises::Backend\n";

sub a_promise {
    my $deferred= deferred;
    $deferred->resolve(1,2,3,4,5);
    return $deferred->promise;
}

timethese(-10, {
    one => sub {
        my $have_result;
        a_promise()->then(sub { a_promise(); })->then(sub { $have_result= 1; });
        while (! $have_result) {
            EV::run EV::RUN_ONCE;
        }
    },
    two => sub {
        my $i = 0;
        a_promise()->then(sub {
            if (++$i == 5) {
                return;
            } else {
                a_promise()->then(__SUB__);
            }
                          });
        while ($i != 5) {
            EV::run EV::RUN_ONCE;
        }
    },
});
ehuelsmann commented 5 years ago

@yanick, anything I can do to advance this PR?

yanick commented 5 years ago

Sorry, I got distracted by other things. I'll try to check this PR tomorrow. Many thanks!

ehuelsmann commented 5 years ago

No hurry. Just checking if I can be off further service.

yanick commented 5 years ago

On my machine t/090-timeout.t hangs forever. Does it pass on yours?

ehuelsmann commented 5 years ago

Indeed it hangs for me too, but only on the EV tests. The cause lies in the call to EV::suspend() instead of to EV::break. I'll push a fix to stop EV from hanging.

ehuelsmann commented 5 years ago

@yanick I rebased to the latest master branch and resolved the hanging of the time-out tests. Sorry that I didn't notice that before. Please reconsider my PR. Thanks!

ehuelsmann commented 5 years ago

@yanick I implemented a small performance improvement: the possibility to reduce the number of writes to the "notification pipe" by eliminating a write when the pipe has already been written to, but the batched callbacks haven't been handled yet.

yanick commented 5 years ago

Woo! It works! I'm prepping a new release as we speak. :-)

ehuelsmann commented 5 years ago

Thanks for accepting my contribution!

yanick commented 5 years ago

:heart: Thanks to you for contributing. :-)