On 04/08/16 19:45, Gustavo Sverzut Barbieri wrote:
> Hi Tom,
>
> Answering both emails here, in order:
>
>> On 04/08/16 17:24, Tom Hacohen wrote:
>>> 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.
>
> indeed, I'll write some examples on the expected usage, would clarify
> a lot indeed.

Thanks.

>
>
>>> 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.
>
> That's exactly what it is, a very thin one.
>
> The blob/binbuf are important optimizations if we care about memory
> consumption and reducing copies, but they are not mandatory. I'll
> insist a bit more on them, but if it's the overall agreement to go
> with buffer copies, then Okay.

I don't see why/how, but maybe I'm missing something, so let me tell you 
what I have in mind, and please correct me if I'm missing anything.

In any networking code you will have:
char buf[MAX_BUF];
read(..., buf, ...);
eo_callback_call(obj, ..., buf, size_of_buf);

Or something similar. Binbuf just means you'll also add it to a binbuf 
and then pass the binbuf instead of buf. So in the most basic case, that 
is one reader of the data (is there any other case??) binbuf is at most 
as good as the example above, and if someone really wants to keep it, he 
just adds it to the binbuf on his own. I don't see how binbuf helps.

>
>
>>> 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).
>
> Problem is usually in the heavy usage, many are solved by using a
> language that helps with reference count for you.
>
> Like in Python, strings are immutable and one always keep references
> to them. In other environments, it's COW, and your reference is kept
> intact...
>
> In C, if we use the easiest API (void* + size_t), we'll pay a price of
> reallocs and memcpy :-/

I think you are pre-optimising here. I don't think that is such a big 
deal regarding performance, and if it is really such a big deal, the 
user can just pass the buffer to be used to the object in a special 
fast-path API, though I don't think this should be norm.

>
>
>>> 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.
>
> it's not a good usability to simply take the "setter" but have no
> effect. If we can mark them as constructor-only, then it's much
> better.
>
> How to do that? Just use constructor { .property_name; } ?

Look at win_type_set in elm.win. The constructor {} directive means a 
function should be used during construction (not necessarily only in 
construction though), and there is another way to say a property is only 
allowed in the constructor. This is done in the C implementation, not 
the .eo declaration though.

>
>
>>> 2. I'd also opt for strings for addresses rather than enum/types and
>>> etc. Feels more correct.
>
> ok, that one I already changed in my local copy to be sent later today.
>
>
>>> 3. I'd also avoid promises.
>
> ok
>
>>> 4. As said, I'd keep it simple, so would avoid binbuf, and your blob
>>> suggestion.
>
> as per above, doable at the price of performance and memory consumption.

Still don't understand.

>
>
>>> 5. There are a few naming issues, some have been pointed out.
>
> what about others?

There was a place you used camelcase, and I don't remember the rest. I 
can have another look once it's more solid. This is a last review kind 
of thing, not now...

>
>
> On Thu, Aug 4, 2016 at 2:14 PM, Tom Hacohen <t...@osg.samsung.com> wrote:
>> I forgot one more thing, and also was asked on IRC for some
>> clarifications about another, so here I am, replying to myself.
>>
>> A comment I forgot to make, which is to something cedric mentioned, and
>> is something that I've said a million times already and I'll say it
>> again because it's so damn important and people should stop doing it.
>> Please do not delete objects implicitly. :( Connection is closed? Don't
>> delete the object, I may steal be dealing with it. Let me eo_unref/del
>> it if I'm still the rightful owner of the object, it's my
>> responsibility. Maybe I still want to introspect it before it's gone?
>> It's my decision to make... If you really think this pattern of
>> autodeletion is useful, add a helper function people can add as a
>> callback that just eo_dels an object. It's that simple.
>
> Not really, I'm trying to use what is the standard and likely i got it
> wrongly. I'm okay to add explicit events and methods to close the
> socket, if that's what I should be doing :-)
>
> I just wanted to use socket.close() if people were expecting to do 
> socket.del().
>
> Note that this shouldn't mess with refcounts, it's more like
> evas_object_del() where you want the object to disappear immediately.
> In this case the socket is not available anymore, so people should
> stop using that after receiving the event.

Yeah, don't mess with refcounts. Don't delete the object when the 
connection dies.

>
>> As for the clarification, I was asked to explain what I meant by
>> "disagree with the concept". What I meant by that, is a "fatter" wrapper
>> than thin. I just want a very thin wrapper, and I disagree with anything
>> that's fatter. :)
>
> As per above, I'm trying to have a very thin layer. The blob/binbuf
> are ways to efficiently use memory in C.

Please explain then, because I don't get it.

>
> BR,
>
> ------------------------------------------------------------------------------
> _______________________________________________
> enlightenment-devel mailing list
> enlightenment-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
>


------------------------------------------------------------------------------
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

Reply via email to