On Thu, 4 Aug 2016 17:24:08 +0100 Tom Hacohen <t...@osg.samsung.com> said:

> Hey Gustavo,
> 
> I am going to give a few comments, though it's really hard to review 
> like this. Although we are creating API here, we don't really care about 
> the API, we care about the usage. Which means, I don't care about how 
> the .eo file looks like, I care about how the .c file that uses it looks 
> like. You haven't provided that, so it means I have to guess, 
> misunderstand and work extra hard only to start a discussion that will 
> include more of the above. I don't really have much experience/knowledge 
> about this topic, but to me, for efficiency and clarity, it seems like 
> the best way to approach it would be to have a very simple wrapper 
> around read()/write() that handles address resolution, timeout, lifetime 
> and loop integration is enough. I don't think we should care about 
> binbuf or any other complicated setups, all we should get is a const 
> string (which is essentially a pointer to the static buffer with which 
> we called "read"), and then decide what we want to do with it on the 
> client side. Calling binbuf_append is not much work.
> All of this should be with simple eo callbacks. As you pointed out, this 
> is really a solved problem, I think most (all?) the frameworks and 
> languages do the basics quite the same. I'd follow suit, because I've 
> never had a bad experience with any. Keep the basics simple, and then 
> wrap as needed with a subclass (like for example, the send file cedric 
> suggested).
> 
> 1. We no longer have a "constructor" just properties we mark as 
> constructing properties and sometimes are only allowed to set on 
> creation. In this case, since the real finalization is actually done in 
> "connect", we don't even need those, and can just have normal code there 
> that checks for those.
> 2. I'd also opt for strings for addresses rather than enum/types and 
> etc. Feels more correct.
> 3. I'd also avoid promises.
> 4. As said, I'd keep it simple, so would avoid binbuf, and your blob 
> suggestion.

i agree with keeping it simple, but binbuf would be the only way to keep it
simple and still have zero copy.

i can see the value of some macros/helper funcs or methods that would create
the binbufs for you from raw data... or maybe strings or something...

> 5. There are a few naming issues, some have been pointed out.
> 
> 
> I can probably find additional specific comments, though to be honest, 
> as said above, I mostly disagree with the concept, and can't really 
> comment without seeing user code, rather than API description.
> 
> --
> Tom.
> 
> On 03/08/16 05:41, Gustavo Sverzut Barbieri wrote:
> > v2 with some fixes.
> >
> > The analysis of competitors at the end was omitted and you can find
> > the updated GIST (diff/highlight):
> >
> > https://gist.github.com/barbieri/5bdf6c49b207543d764435d2494361ba
> >
> > Fixed:
> >  - changed base class from Eo.Base to Efl.Loop_User
> >  - Returns instead of @out parameters
> >  - added "drain" event to BaseSocket
> >  - fixed naming: local_address -> address_local, same for remote
> >  - added send_buffer_usage property to BaseSocket
> >  - added receive_buffer_size and send_buffer_size to BaseSocket
> >  - added address_connected to Socket (client)
> >  - added address and client_limit to Server
> >  - addresses of a socket contain port, thus SocketAddress not just Address
> > type.
> >
> > Pending:
> >  - Address as strings?
> >  - Use promise<> for send?
> >  - Constructors without parameters, use properties and finalize?
> >  - How to have properties to be usable only in constructor->finalize
> > timeline?
> >  - Drop blob and use binbuf?
> >
> >
> >
> > /*
> >
> > This is a draft Eo model to replace ecore_con_{server,client}.
> >
> > It will address the same usage, but with a new API that is closer
> > to what other projects do, this will allow easier transition to
> > EFL, without loosing any feature.
> >
> > At the of this file you can see a detailed analysis of Ecore_Con.h and
> > competitors such as node.js, golang, qt and soletta.
> >
> > NOTE: SSL and Proxy are left out from the initial discussion, Qt is
> > the only one to provide API for proxy and they are unused in our
> > repository. SSL is usually handled via a subclass, not an upgrade from
> > the existing connection.
> >
> > NOTE: this Eo is not valid or fully complete, it's used as a starting
> > base.
> > */
> >
> > import eina_types;
> >
> > struct Efl.Net.Address {
> >     family: Efl.Net.Family;
> >     address: union { in, in6, bt... };
> > }
> >
> > struct Efl.Net.SocketAddress {
> >     address: Efl.Net.Address;
> >     port: uint16;
> > }
> >
> > // TODO: string->Efl.Net.Address resolver.
> >
> > /* see http://solettaproject.github.io/docs/c-api/structsol__blob.html
> >  *
> >  * The idea is to allow an immutable memory to be reference counted and have
> >  * parent.
> >  *
> >  * Children reference their parent and releases the referece when they
> >  * die, this allows "windowing" or "slicing" of memory to be handled
> >  * easily, such as a subsurface keeping the parent framebuffer alive,
> >  * or a "line" keeping the backing mmap'ed file.
> >  *
> >  * Free function allows custom release, such as unloading an image or
> >  * munmap of a text file.
> >  *
> >  * This plays well with bindings, keep the object alive (inc ref) and
> >  * once the blob dies release them (dec ref).
> >  */
> > struct Eina.Blob {
> >     mem: void_ptr;
> >     parent: Eina.Blob;
> >     size: size;
> >     refcount: int;
> >     free: Eina_Blob_Free_Cb;
> > }
> >
> > /*
> >  * Using 'Net' instead of 'Network' since all others use a shorter
> >  * prefix. Also allows to have a side-by-side implementation without
> >  * the need to immediately convert the all users and allows changing
> >  * paradigm of the new class -- of course legacy wrappers will be kept
> >  * for old users.
> >  */
> > abstract Efl.Net.BaseSocket (Efl.Loop_User) {
> >     [[Abstract class representing a network connection socket]]
> >
> >     events {
> >         received @hot: Eina.Binbuf; [[data is available to read,
> >                             if consumed use eina_binbuf_remove().]]
> >         sent: Eina.Blob, error; [[data specified by blob was sent,
> >                                  if error != 0, contains errno]]
> >
> >         drain: void; [[all pending data was sent.]]
> >         error: int; [[errno]]
> >     }
> >
> >     methods {
> >         send @virtual_pure {
> >             [[Queue data to be sent to remote]]
> >             params {
> >                 @in data: Eina.Blob; [[data to queue for sending]]
> >             }
> >
> >             return: int; [[0 on success, ENOSPC if no space left]]
> >
> >             /* can provide a helper that sends memory, creating the
> >                blob internally, such as static-const (no free) or
> >                allocated memory (call free)
> >             */
> >
> >             // TODO: promise<size, Eina_Error>?
> >         }
> >
> >         flush @virtual_pure {
> >             [[Try to send as much as data without blocking]]
> >             /* NOTE: changes behavior from current flush() that
> >              * really blocks!
> >              */
> >             return: bool; [[true if all data was sent]]
> >         }
> >
> >         @property address_local {
> >             [[the local IP or unix-local address. Analogous to getsockname
> > ()]] get @virtual_pure {
> >             }
> >             values {
> >                 address: Efl.Net.SocketAddress; // TODO: string?
> >             }
> >         }
> >
> >         @property address_remote {
> >             [[the remote IP or unix-local address. Analogous to getpeername
> > ()]] get @virtual_pure {
> >             }
> >             values {
> >                 address: Efl.Net.SocketAddress; // TODO: string?
> >             }
> >         }
> >
> >         /* TO BE REMOVED:
> >          * it seems the usage is to steal the fd to do something
> >          * else or set some flags. If so we could expose some
> >          * primitives such as fcntl_add(type, flags) and
> >          * fcntl_del(type, flags) as well as a destructor that
> >          * will return the stolen fd (destroys the object and
> >          * keeps fd alive).
> >          */
> >         /* @property internal_fd { ... }*/
> >
> >         @property connected {
> >             [[if the socket is still connected.]]
> >             get @virtual_pure {
> >             }
> >             values {
> >                 connected: bool;
> >             }
> >         }
> >
> >         @property send_buffer_usage { /* maybe pending_send_bytes? */
> >             [[how many bytes are queued to be sent.
> >               See property send_buffer_size.]]
> >             get @virtual_pure {
> >             }
> >             values {
> >                 bytes: size;
> >             }
> >         }
> >
> >         @property receive_buffer_size {
> >             [[Amount of bytes to use when receiving data.
> >               0 is unlimited, > 0 is fixed size.]]
> >             get @virtual_pure {
> >             }
> >             values {
> >                 bytes: size;
> >             }
> >         }
> >
> >         @property send_buffer_size {
> >             [[Amount of bytes to use when sending data.
> >               0 is unlimited,
> >               > 0 is an upper limit of queued bytes before
> >               send() returns ENOSPC]]
> >             get @virtual_pure {
> >             }
> >             values {
> >                 bytes: size;
> >             }
> >         }
> >
> >         @property timeout { /* this usually doesn't change,
> >                                should be constructor-only */
> >             [[timeout to close the connection.]]
> >             values {
> >                 timeout: double;
> >             }
> >         }
> >     }
> > }
> >
> >
> > class Efl.Net.Socket (Efl.Net.BaseSocket) {
> >     [[This is a client socket that is created and connected to a
> >       specific server given to the constructor]]
> >
> >     /* TODO: if address changes to string, add: resolved */
> >     /* events { */
> >     /*     resolved: Efl.Net.Address; [[the connecting address was
> > resolved]] */ /* } */
> >
> >     methods {
> >         connect {
> >             [[The constructor that connects the socket to a given server]]
> >             params {
> >                 @in type: Efl.Net.Socket.Type;
> >                 @in address: Efl.Net.SocketAddress; // TODO: string?
> >                 @in timeout: double;
> >             }
> >         }
> >
> >         @property address_connected {
> >             [[The address used to create and connect this socket. It
> >             is the unresolved address, see the property address_remote
> >             for the actual resolved address]]
> >             get @virtual_pure {
> >             }
> >             values {
> >                 address: Efl.Net.SocketAddress; // TODO: string?
> >             }
> >         }
> >     }
> >
> >     constructors {
> >         .connect;
> >     }
> > }
> >
> > class Efl.Net.Server (Efl.Loop_User) {
> >     events {
> >         connection @hot: Efl.Net.BaseSocket; [[new connection was accepted]]
> >         error: int; [[errno]]
> >         /* no disconnect, listen on each individual connection */
> >     }
> >
> >     methods {
> >         constructor {
> >             params {
> >                 /* these could be converted into a struct to scale
> >                  * gracefully and allow just some members to be set,
> >                  * like only timeout without setting client_limit or
> >                  * reject_excess_clients.
> >                  */
> >                 @in type: Efl.Net.Socket.Type;
> >                 @in address: Efl.Net.SocketAddress; // TODO: string?
> >                 @in client_limit: int;
> >                 @in reject_excess_clients: bool;
> >                 @in timeout: double;
> >                 @in bind_flags: Efl.Net.Socket.Bind.Flags; /* reuse addr..*/
> >                 @in accept_flags: Efl.Net.Socket.Accept.Flags; /* just
> > SOCK_CLOEXEC */
> >             }
> >         }
> >
> >         @property address {
> >             [[the local IP or unix-local address used to create and
> > listen.]] get @virtual_pure {
> >             }
> >             values {
> >                 address: Efl.Net.SocketAddress; // TODO: string?
> >             }
> >         }
> >
> >         @property client_limit {
> >             [[number of concurrent clients accepted by this server]]
> >             get @virtual_pure {
> >             }
> >             values {
> >                 client_limit: int;
> >                 reject_excess_clients: bool;
> >             }
> >         }
> >
> >         @property timeout { /* this usually doesn't change,
> >                                should be constructor-only */
> >             [[timeout to close the connection.]]
> >             values {
> >                 timeout: double;
> >             }
> >         }
> >     }
> >
> >     constructors {
> >         .constructor;
> >     }
> > }
> >
> >
> 
> 
> ------------------------------------------------------------------------------
> _______________________________________________
> enlightenment-devel mailing list
> enlightenment-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
> 


-- 
------------- Codito, ergo sum - "I code, therefore I am" --------------
The Rasterman (Carsten Haitzler)    ras...@rasterman.com


------------------------------------------------------------------------------
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity 
planning reports. http://sdm.link/zohodev2dev
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

Reply via email to