p-alik / perl-Gearman

9 stars 10 forks source link

Gearman::Worker minor optimizations #18

Closed esabol closed 7 years ago

esabol commented 7 years ago

I'm not sure if the Perl compiler already optimizes these these things or not, but I suspect not. The following simple optimizations could be made:

  1. Replace

    sub _rc {
    return Gearman::Util::pack_req_command(@_);
    }

    with

    *_rc = \&Gearman::Util::pack_req_command;

    A simple change that should improve efficiency.

  2. _send() is a little different though. The second argument is a SCALAR, but Gearman::Util::send_req() wants a REF.

    sub _send {
    my ($jss, $req) = @_;
    return Gearman::Util::send_req($jss, \$req);
    }

    Passing a REF is more efficient, especially with very large requests.

Two possible solutions:

a. Modify Gearman::Util::send_req() to work with the second argument being either a REF or a SCALAR. Then, replace "sub _send { ...}" with

*_send = \&Gearman::Util::send_req_command;

Then, change all the second arguments in the _send() calls to pass the request as a reference.

b. Mark _send() as deprecated (but keep it in case third-party modules or subclasses use it) and change all _send() calls to Gearman::Util::send_req() with the second argument passed as a reference.

What do you think?

p-alik commented 7 years ago

Both are done in upstream. _send is an internal sub. It was introduced in v2.003_001 in February. Therefore the sub was rewritten with reference to Gearman::Util::send_req

esabol commented 7 years ago

Let me know when the commit makes it here to GitHub, and I'll take a look. Thanks!

p-alik commented 7 years ago

I beg your pardon, Ed. The changes are in upstream branch. your can see diffs:

esabol commented 7 years ago

Ah, that's what you meant by upstream. Those changes all look good to me!