websocket-libev

[Xaraknid]

I like this version of websocket. I used it like that, but didn't like the workaround needed to be able to have over 1024 concurrent connection.

So I modified the run() to use libev library instead of select().

I share with you the modification. Feel free to add it or not.

[ghedipunk]

I'll keep this open, but with no plans to merge as-is.

Unfortunately, the PHP Ev extension is a PECL extension that depends on the LibEv library, which is not a typical configuration for many distros. I'm a little wary to depend on any PECL extensions, so that the script can be compatible out-of-the-box with as many systems as possible.

I have some extra time coming up, where I can work on tying performance boosting changes like this into optional modules.

[Xaraknid]

I understand and that's why I called it wesocket-libev, because I want it to be add as an option. As you mentionned it's not an out-of-the-box solution it need to install the extension and some people can't.

You can have both websocket and websocket-libev on the project. Easy methed could be : -modify line 4 of testwebsock.php "require_once('./websockets.php');" with

if (extension_loaded('ev')) require_once('./websockets-libev.php'); else require_once('./websockets.php');

Also warn that they have an option to use event-base method but need to install ev extension.

Now that I understand more how work github I'll add this file to my directory with the deframe fails fix.

[ghedipunk]

Having much of the same code in two separate files creates a maintenance issue. It's a decent short-term fork, which I fully support, but really couldn't be merged back into the trunk.

Rather, I've been working on modularizing the code, pulling apart the tight coupling wherever I can find a seam, to allow for a swap-in-place system using factories.

This way, we can have the naive tight loop that uses the built-in PHP socket functions be the default, with modules that implement the same functionality with the ev and libevent PECL extensions... Then, when we fix a bug, such as the one you fixed by implementing split_packet, we only fix it in one place in the deframing system, leaving the different event systems alone to do their work in blissful isolation.

Again, good improvement. I just want to merge it in correctly.

[Xaraknid]

I missunderstand you first time. You are totally right it must be in same file, having to fix issue in 2 place can be a pain.

Just for clarification Ev use libev PECL extension http://php.net/manual/fr/book.ev.php. libevent PECL extension is for Libevent http://php.net/manual/fr/book.libevent.php.

I think it will not be too hard to merge both method into same file. I think the best place to fork both method will be in run(). For the same reason you mentionned both method should use cb_read() to avoid duplicate code.

[ghedipunk]

I understand the difference between ev (libev) and libevent... I included both in my explanation to show that I want things that take 3rd party resources to be interchangeable with their alternatives, like MySQL and MariaDB, or even PostgreSQL, Oracle, MS SQL Server, etc.

As for where to put the different event managers, I'm working on splitting the application into several different classes which will be replaceable within a factory. As long as each class implements the same interfaces, the new classes can fulfil the promises of the classes that they're replacing.

Then, we would have a global configuration that would keep track of which class handles each responsibility... For example:

File: config/config.ini

[factory overrides]
# Override the default EventHandler class to be and instance of \EventHandlers\Libev
EventHandler = "\Core\EventHandlers\Libev"

File: core/eventhandlers/libev.php

<?php
namespace \Core\EventHandlers

class Libev implements \Core\Interfaces\EventHandler
{
    public function run() { ... }
}

File: ws.php

<?php
...
$evh = $factory->create('EventHandler'); // Returns instance of \Core\EventHandlers\Libev based on ini settings
$evh->run();

(Note: Probably not the final namespaces or directory structure)

[Xaraknid]

I missread your intention. Indeed this method will be better.

In the same time I made a version of websocket-libev put on my directory https://github.com/Xaraknid/PHP-Websockets/blob/master/websocket-libev.php that can use both method (default and libev) smoothly. If that can help you create eventhandler.

diference between these two are default use $sockets and libev use $watchers. function affected by this 2 method run() connect() and disconnect().

[Xaraknid]

I take a look on the developement branch, Here is just my opinion. I did not have the big picture you have over the project also in the end you are the original author of this and have all authority on what you want this to be.

At first glance it's seem to me to be overkill and complex. As for polling library the list is fairly short (socket,libev,libevent,libuv(experimental),any other ). As CLI environement we will only load once all class and required depency as opposed in SAPI environement where each page load each time and can need different dependency.

The reason I take libev is because it work on same level of TCP control than socket. So they are highly compatible without too much work. I be able to make a version where both method are supported in same class. No tweaking, configuring, coding requiered for dev to fully use any of the method other than having to install the library of course.

In other hand libevent and libuv work on a higher level of TCP control than socket. I did not dig very far in libevent, implementing it will maybe affect more method or not but surely doable to do it easily and hidely from person wanted to use this project.

Myself I try to respect this 3 primary rules when I coding ; -If it ain't broke don't fix it. -If it doesn't cause a problem it doesn't need a solution. -Keep It Simple,

I'm not sure your solution will be simple, The easiest way if you want to keep apart the forking method can be by overriding the class with "extends".

Still I can be wrong on that. I'm looking forward to see the final product. Good luck!

[amb3rl4nn]

I would agree with Adam on the Factory process. It would really not change anything in regards to somebody implementing libev and make this library pretty flexible. For example libev is not available on windows from what I just read and my application happens to be hosted on a windows box. The factory approach makes sure that you can update to a master release and all of your current code will never be affected.

Personally I love the fact that this library is super small as it is but even currently as its written it is the "main" process. And in my case I don't want the socket to ever stop my process. I load it inside of my server loop and had to make physical changes to the source which isn't a big deal but not the ideal method for supporting every bodies needs. Next time somebody patches I will have to make those same changes again.

Just some thoughts!

On Wed, Apr 15, 2015 at 8:23 AM, Xaraknid notifications@github.com wrote:

I take a look on the developement branch, Here is just my opinion. I did not have the big picture you have over the project also in the end you are the original author of this and have all authority on what you want this to be.

At first glance it's seem to me to be overkill and complex. As for polling library the list is fairly short (socket,libev,libevent,libuv(experimental),any other ). As CLI environement we will only load once all class and required depency as opposed in SAPI environement where each page load each time and can need different dependency.

The reason I take libev is because it work on same level of TCP control than socket. So they are highly compatible without too much work. I be able to make a version where both method are supported in same class. No tweaking, configuring, coding requiered for dev to fully use any of the method other than having to install the library of course.

In other hand libevent and libuv work on a higher level of TCP control than socket. I did not dig very far in libevent, implementing it will maybe affect more method or not but surely doable to do it easily and hidely from person wanted to use this project.

Myself I try to respect this 3 primary rules when I coding ; -If it ain't broke don't fix it. -If it doesn't cause a problem it doesn't need a solution. -Keep It Simple,

I'm not sure your solution will be simple, The easiest way if you want to keep apart the forking method can be by overriding the class with "extends".

Still I can be wrong on that. I'm looking forward to see the final product. Good luck!

— Reply to this email directly or view it on GitHub https://github.com/ghedipunk/PHP-Websockets/pull/25#issuecomment-93366618 .

[ghedipunk]

I do fully intend to keep the current code in place and continue to maintain it indefinitely.

There are several issues with the current system.

1) You can't fork, change, and re-merge effortlessly. An upgrade to the latest bugfix version should overwrite all core files completely, but still leave a local installation's modifications in place and working correctly. (See amb3rl4nn's comment above: They shouldn't have to re-patch in order to implement the latest version.)

2) You can't unit test. Every piece of functionality depends on the whole and complete application to be fully loaded, so you can't load a class in isolation, feed it carefully manipulated data, and ensure that it always remains consistent in how it fulfils the promises it implements. The only way to test is to run the application, feed it data through its real-world interfaces, and hope that you can trigger fault conditions at that point.

3) The user (that's you, the developer who is using the server to support their own users) can not change any part of the program without understanding the entirety of it. With a modular approach, someone implementing a new way to read from the TCP socket doesn't have to know how the WebSocketUser class works, they just have to know what interfaces to implement, and shouldn't need to even look at the baseline class. Yes, forcing your users to have intimate knowledge of the system is a bug. A user interface is like a joke: the more you have to explain it, the worse it is.

4) This is rife with breaking OOP principles. While this is more of a philosophical approach than a bug, in its current state it may have been better written procedurally. The way that inheriting is set up in this case breaks encapsulation. There is only one case of separation of concerns out of many points where separation of concerns is clearly called for. With one huge master class, everything might as well be global because all of the application state can always be changed directly. I'm sure someone with more time and who is more pedantic than me could go on and on.

5) It would be very difficult to shoehorn it into a large CMS or framework, like Magento, Drupal, or Symfony. This is actually my personal prime motivation... issues like multiple frames inside the same packet and libev have just been examples of why I need to tackle the switch to a modular, factory-based approach sooner rather than later.

[Xaraknid]

Thanks @ghedipunk for this quick, clear and detailed answer. The first sentence was definately my biggest concern I'm happy to learn that you will keep that method in some way. I'm sure I was not the only one to prefer this as it's merge easily into my project.

As for some will prefer the next method you are currently developping. That all valid point that enforce the way you want it to be.

@amb3rl4nn : it's sad that libev was not a supported on windows box, that left you only one alternative for select() polling as libevent. Also I'm really curious how you patch it, because you don't want socket to block. I guess you make sure all sockets are in non blocking mode and you remove the wait untill modification on select polling like that :

@socket_select($read,$write,$except,null);

to

$timeout=10; in seconds
@socket_select($read,$write,$except,$timeout);

you can even put it at 0 seconds, but you will have to keep an eye on CPU usage as your server will be in constant loop without sleep.