Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Phpiredis + predis 1.0 performance issue #180

Closed
ad34 opened this issue Jun 17, 2014 · 5 comments
Closed

Phpiredis + predis 1.0 performance issue #180

ad34 opened this issue Jun 17, 2014 · 5 comments
Labels

Comments

@ad34
Copy link

ad34 commented Jun 17, 2014

Not sure if the issue is in predis or phpiredis itself, but I ve got a performance issue with predis and phpiredis and I don't reproduce the issue without the phpiredis connector

script to reproduce:

array('tcp' => 'Predis\Connection\PhpiredisStreamConnection') ); $predis = new Predis\Client($parameters, $options); $t = microtime(true); $c = $predis->get('communities'); echo "time to fetch communities : ".(microtime(true)-$t); ?>

some call , like 1 / 3 or 1/4 takes 60 seconds

time to fetch communities : 60.002037643

if I remove the 'connections' option the script runs fast all time

time to fetch communities : 0.0055761337280273

@nrk
Copy link
Contributor

nrk commented Jun 18, 2014

Did you try disabling persistent connections and see if the issue is still present?

@ad34
Copy link
Author

ad34 commented Jun 18, 2014

Yes, the issue is still present without persistant connexion

@nrk
Copy link
Contributor

nrk commented Jun 18, 2014

Try replacing this line in lib/Predis/Connection/PhpiredisStreamConnection.php with the following one:

$buffer = stream_socket_recvfrom($socket, 4096);

And see if anything changes.

@ad34
Copy link
Author

ad34 commented Jun 18, 2014

Looks like it fixes the issue , nice !

@nrk
Copy link
Contributor

nrk commented Jun 19, 2014

Actually I'm surprised to see I used fread there to begin with due to how the streaming parser of phpiredis works, anyway I'll take some time to test the change before pushing it into the repository. In the meanwhile you can stick with it and eventually leave a new comment if you find it unstable.

Thanks!

nrk added a commit that referenced this issue Jun 23, 2014
Similarly to the socket-ext based connection using phpiredis, in our
stream based PhpiredisStreamConnection class we should read data from
the stream using stream_socket_recvfrom() instead of fread() because
the latter could block until a timeout is reached when the read buffer
contains less data then the specified length.

IMPORTANT: stream_socket_recvfrom() bypasses stream wrappers which
means that TLS/SSL, as requested by PR #158, won't ever work with
this connection class as the function returns the original encrypted
bytes.

This commit fixes issue #180.
nrk added a commit that referenced this issue Jun 23, 2014
Similarly to the socket-ext based connection using phpiredis, in our
stream based PhpiredisStreamConnection class we should read data from
the stream using stream_socket_recvfrom() instead of fread() because
the latter could block until a timeout is reached when the read buffer
contains less data then the specified length.

IMPORTANT: stream_socket_recvfrom() bypasses stream wrappers which
means that TLS/SSL, as requested by PR #158, won't ever work with
this connection class as the function returns the original encrypted
bytes.

This commit fixes issue #180.
@nrk nrk closed this as completed Jun 23, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

2 participants