Export to GitHub

php-amqplib - issue #3

Performance Optimizations


Posted on Mar 3, 2009 by Helpful Elephant

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?

Attachments

Comment #1

Posted on Mar 13, 2009 by Happy Dog

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); }

Comment #2

Posted on Mar 13, 2009 by Helpful Elephant

@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.

Attachments

Comment #3

Posted on Mar 14, 2009 by Happy Lion

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?

Comment #4

Posted on Mar 14, 2009 by Helpful Elephant

@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.

Comment #5

Posted on Mar 14, 2009 by Happy Lion

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.

Comment #6

Posted on Oct 3, 2009 by Quick Rhino

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

Attachments

Comment #7

Posted on Mar 21, 2010 by Happy Lion

Lyolik please review these bugs.

Status: New

Labels:
Type-Defect Priority-Medium