async-interop / event-loop

An event loop interface for interoperability in PHP.
MIT License
170 stars 9 forks source link

Common Driver Implementations #151

Open trowski opened 7 years ago

trowski commented 7 years ago

The Driver interface leaves no room for differing behaviors between implementations. Once a particular vendor has written performant Driver implementations for various backends there is little reason for another vendor to do the same. Changes in behavior between Driver implementations render them non-interoperable. Perhaps then this group should also offer Driver implementations, dissociating Driver implementations from any particular vendor.

amphp/loop contains highly optimized Driver implementations based on the native stream_select(), libevent (ext-event), libev (ext-ev), and libuv (ext-uv). Should this library be migrated to be a part of this project?

kelunik commented 7 years ago

Huge :+1:, makes using the loop way easier. People can just require async-interop/event-loop and things work. I'd still keep the factory mechanism, so people can still write their own implementations, maybe with custom underlying extensions.

joshdifabio commented 7 years ago

What if we ended up with two versions of the uv driver which worked with different versions of the uv extension? Perhaps the drivers which rely on extensions should be packaged separately and we should just include a stream_select loop?

Edit: I suppose in that case we could instead have two uv drivers in this package, with the correct one picked based on the extension version. Or the driver could simply be designed to switch behaviour based on the extension version. Maybe this isn't an issue.

kelunik commented 7 years ago

@joshdifabio This package will take care of it then and add a new driver, just like any other package would. Not including the extension based drivers is a no-go IMO, as it effectively doesn't bring any value then.

joshdifabio commented 7 years ago

This would certainly simplify usage. It does raise the question of what the difference between this package and, say, amphp/loop would actually be. Clearly replacing vendor-specific loop packages would be the intention here but I wonder if React and others would actually be on board with this.

kelunik commented 7 years ago

@joshdifabio React adapting the interop loop is highly unlikely. Everything we can do is provide an adapter so every interop library / application can use any React library.

It does raise the question of what the difference between this package and, say, amphp/loop would actually be.

None, they're just merged then, it's the exact proposal.

joshdifabio commented 7 years ago

Okay. I think I'm on board as well.

trowski commented 7 years ago

Ping @bwoebi @jsor @sagebind @WyriHaximus @AndrewCarterUK

sagebind commented 7 years ago

I'm in favor. Only issue I see is from a community perspective it might seem like a conflict of interest. It might send the wrong idea that implementing the standard is discouraged, since we would point to using our implementation instead.

kelunik commented 7 years ago

@sagebind Implementing the already supported backend extensions again is indeed unnecessary. If there's room for improvement, submit a PR to this repository instead. Custom implementations will be allowed, but not the usual way of doing things.

kelunik commented 7 years ago

Also ping @cboden and @clue.

rdlowrey commented 7 years ago

:+1: ... particularly if including a react adapter is a feasible option

trowski commented 7 years ago

@rdlowrey I wouldn't include it in this package, but having a separate package under async-interop for a React adapter would be a possibility.