Open GoogleCodeExporter opened 9 years ago
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 even...@gmail.com
on 13 Mar 2009 at 6:43
@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 sgmur...@gmail.com
on 13 Mar 2009 at 8:30
Attachments:
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 kroko...@gmail.com
on 14 Mar 2009 at 8:34
@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 sgmur...@gmail.com
on 14 Mar 2009 at 4:17
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 kroko...@gmail.com
on 14 Mar 2009 at 10:41
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 bernho...@gmail.com
on 3 Oct 2009 at 3:42
Attachments:
Lyolik please review these bugs.
Original comment by kroko...@gmail.com
on 21 Mar 2010 at 9:17
Original issue reported on code.google.com by
sgmur...@gmail.com
on 3 Mar 2009 at 3:58Attachments: