Performance Optimizations

[GoogleCodeExporter]

I did a little profiling and optimization of the code, particularly for
writing messages. In my usage I was able to yield approximately a 280%
increase in speed with these changes.

I hope to look into this more, but here are some ideas for further
optimization...

I noticed AMQPConnection->write() appears to be called approximately 3
times for every message that is written. Is this necessary, or could we
reduce this to a 1:1 ratio and write one batch request for each message?

AMQPWriter::chrbytesplit() is still slow. Is it possible to pack() a 64-bit
integer instead of using this function?

Original issue reported on code.google.com by [email protected] on 3 Mar 2009 at 3:58

Attachments:

• performance.patch

[GoogleCodeExporter]

I was going through and performing optimizations of the library and came up 
with many
of the same changes in the provided patch.  Removing the hexdump call on every 
write,
removing the count() from flushbits and changing write_long() to use a simple 
pack()
are the most important improvements.

As far as write_longlong() goes, I got a significant speed improvement with 
this,
which I believe should be a portable change:

public function write_longlong($n)
{
    $this->flushbits();
    $c = 4294967296; // 2^64
    $b = bcmod($n, $c);
    $a = bcdiv($n, $c);
    $this->out .= pack('N', $a);
    $this->out .= pack('N', $b);
}

Original comment by [email protected] on 13 Mar 2009 at 6:43

• Added labels: ****
• Removed labels: ****

[GoogleCodeExporter]

@evendir Thanks for sharing. I just tested that function and I got a significant
performance increase as well. Nice work!

I've attached an updated patch that also includes some other optimizations I 
made.

Original comment by [email protected] on 13 Mar 2009 at 8:30

• Added labels: ****
• Removed labels: ****

Attachments:

• performance.patch.2

[GoogleCodeExporter]

I am concerned a bit, now new write_longlong() would work on 32-bit systems.
In particular the following line maybe be treated as 'signed'. BTW it is 2^32, 
not 2^64 :)

    $c = 4294967296; // 2^64

Could somebody please verify that this function works as expected on both 32 
and 64 bit systems?
Maybe we need to make a simple unit test for things like this? 

Original comment by [email protected] on 14 Mar 2009 at 8:34

• Added labels: ****
• Removed labels: ****

[GoogleCodeExporter]

@krokodil I agree, it's important for these things to be tested on both 32 & 64 
bit
systems. I'm using it on a 32 bit system and it appears to work without problem.

One thing I failed to adjust in the updated patch could have both a performance 
and
portability impact: $c = 4294967296; should be $c = '4294967296';. Aside from 
the
fact that bcmod() and bcdiv() expect string inputs (and perform better when 
inputs
are such), I think declaring $c as a string would circumvent PHP's funky 
internal
integer datatype.

Original comment by [email protected] on 14 Mar 2009 at 4:17

• Added labels: ****
• Removed labels: ****

[GoogleCodeExporter]

I have added unit tests to the project. To run from command line:

cd tests
php all_tests.php

I have tested and current code base passes all tests on both 32 and 64 bit 
system.
Last patch provided here fails unit tests:

deco /Users/lord/src/php-amqplib/tests> php all_tests.php 
PHP-AMQP tests (64bit)
1) Equal expectation fails at character 4 with [994284929023] and 
[994294967296] at [/Users/lord/src/php-
amqplib/tests/wire_format_test.php line 42]
        in testWriteLong
        in AMQPWriterTests
        in wire_format_test.php
FAILURES!!!

We need a new patch which passes all tests.


P.S. Feel free to add more tests cases.

Original comment by [email protected] on 14 Mar 2009 at 10:41

• Added labels: ****
• Removed labels: ****

[GoogleCodeExporter]

This version of the patch passes the tests on macos darwin 32bit and 
Centos-Linux 64bit.

You can also find it at http://github.com/bkw/php-amqp/commits/issue3

I've also added some more tests (moretests.patch)

regards,
  bkw

Original comment by [email protected] on 3 Oct 2009 at 3:42

• Added labels: ****
• Removed labels: ****

Attachments:

• performance.patch.3
• moretests.patch

[GoogleCodeExporter]

Lyolik please review these bugs.

Original comment by [email protected] on 21 Mar 2010 at 9:17

• Added labels: ****
• Removed labels: ****