Lazy socket-binding and pipelining, for better predis support

[stokes3452]

Adds the ability to create a phpiredis connection from an existing socket. Also adds the ability to append commands to the redisContext (which will be flushed on demand before eventual reads), and read them back one at a time. When these are dropped into Predis's PhpiredisStreamConnection, performance increases dramatically.

[nrk]

Hi @stokes3452,

Just give me a a couple of days and I'll be sure to get back to you, I'm currently a bit busy but I'm interested in these changes!

[nrk]

I finally have some time for phpiredis, sorry for the wait! I have only a few notes for now:

1. How about renaming phpiredis_create_from_stream() to phpiredis_import_stream() along the lines of PHP's function socket_import_stream?
2. Instead of passing a reader resource to phpiredis_create_from_stream() just to be able to set custom status and error handlers, wouldn't it be better to instantiate a reader resource internally and associate it to the connection in both s_create_connection and phpiredis_create_from_stream() and add a new phpiredis_get_reader() function that returns the underlying reader resource? This would make it possible to add custom handlers even when using the usual phpiredis_connect() function. Thoughts?
3. Can you update tests/027.phpt to follow the new structure of the test suite?

[nrk]

About the 2nd point I think I understand now why you decided to go with an optional $reader argument for phpiredis_create_from_stream(): in Predis we create and set up a phpiredis_reader resource without having a valid PHP stream which is created at a later stage due to the lazy behavior of connection objects.

I still prefer my proposal (avoiding the optional $reader argument in phpiredis_create_from_stream()), as for Predis we can simply modify Predis\Connection\PhpiredisStreamConnection to do the following:

protected function createResource()
{
    $stream = parent::createResource();

    $phpiredis = phpiredis_create_from_stream($stream);

    $reader = phpiredis_get_reader($phpiredis);
    phpiredis_reader_set_status_handler($reader, $this->getStatusHandler());
    phpiredis_reader_set_error_handler($reader, $this->getErrorHandler());

    return $phpiredis;
}

Now I wonder if it could be useful to have a phpiredis_set_reader() function just to make the interface more complete, after all we have the ability to create a reader resource in userland code by using phpiredis_reader_create().

[nrk]

I just noticed that redisConnectFd() has been added only recently in hiredis but there's no tagged release including it which is a problem since I was thinking of importing these changes to phpiredis 1.0.0, I guess this makes it not possible which is a pity. Worth having it anyway after tagging phpiredis, so we can ship it in a future release.

[nrk]

Ping @stokes3452?

[stokes3452]

Sorry, got busy and completely lost track of this. Commit incoming, with function rename and a set_reader function instead of the optional $reader parameter. I went with an optional set_reader call over an optional get_reader call, so that people who don't want to have a reader associated with their context don't have to.

What's the new structure in the test suite?

[nrk]

Don't worry @stokes3452 I just pinged you to keep the PR alive, I too got suddenly busy again so I couldn't reply sooner :-)

The structure and layout of the test suite has been recently changed in the master branch to make it a bit more tidy, you should probably merge these changes back into your branch but I can do it myself when merging locally.

On a related note, I've informally asked if there were plans to tag a new release of hiredis given the good amount of changes since the last release (and one of those changes is the very redisConnectFd() function) but unfortunately I didn't get any response yet. I'd really love to get your PR into a stable 1.0 release of phpiredis because it's a great addition and Predis would benefit from it, this time I'll try asking again by opening an issue on the hiredis repository.