Menu
▾
▴
Re: [E-devel] E SVN: tasn trunk/eina/src/include
|
From: Gustavo S. B. <bar...@pr...> - 2011-05-09 17:32:16
|
On Mon, May 9, 2011 at 2:03 PM, Cedric BAIL <ced...@fr...> wrote:
> On Tue, May 3, 2011 at 8:19 PM, Gustavo Sverzut Barbieri
> <bar...@pr...> wrote:
>> On Tue, May 3, 2011 at 6:17 AM, Cedric BAIL <ced...@fr...> wrote:
>>> On Sat, Apr 30, 2011 at 11:59 PM, Gustavo Sverzut Barbieri
>>> <bar...@pr...> wrote:
>>>> On Sat, Apr 30, 2011 at 1:09 PM, Tom Hacohen <to...@st...> wrote:
>>>>> On Sat, Apr 30, 2011 at 5:59 PM, Gustavo Sverzut Barbieri
>>>>> <bar...@pr...> wrote:
>>>>>> On Fri, Apr 29, 2011 at 3:54 PM, Enlightenment SVN
>>>>>> <no-...@en...> wrote:
>>>>>> > Log:
>>>>>> > Eina refcount: Wrap EINA_REFCOUNT_UNREF with do {} while(0).
>>>>>> > #define EINA_REFCOUNT_UNREF(Variable, Free_Callback) \
>>>>>> > - if (--((Variable)->__refcount) == 0) \
>>>>>> > - Free_Callback(Variable);
>>>>>> > + do \
>>>>>> > + { \
>>>>>> > + if (--((Variable)->__refcount) == 0) \
>>>>>> > + Free_Callback(Variable); \
>>>>>> > + } \
>>>>>> > + while (0)
>>>>>>
>>>>>> maybe think of a way to set Free_Callback automatically? Spread this
>>>>>> all around the code will be bad?
>>>>>
>>>>> I don't really agree. If the free callback is type specific then you'll get
>>>>> a compilation warning
>>>>> when doing something wrong and how different is it from just calling our
>>>>> custom
>>>>> unref_my_special_item?
>>>>>
>>>>> I.e it's
>>>>> EINA_REFCOUNT_UNREF(item, free_my_item);
>>>>> v.s
>>>>> unref_my_item(item);
>>>>>
>>>>> Both are error-prone the same way.
>>>>>
>>>>>
>>>>> Coming to think about it, the current way is not good because it breaks
>>>>> inheritance.
>>>>> Here's an example of something that will break:
>>>>>
>>>>> struct parent { EINA_REFCOUNT; int foo; };
>>>>>
>>>>> struct child { struct parent p; int bar; };
>>>>>
>>>>> there's no sane way to do
>>>>> struct child item;
>>>>> ...
>>>>> EINA_REFCOUNT_UNREF(item, free_my_item);
>>>>> because child->__refcount doesn't exist and passing &child->parent instead
>>>>> will cause
>>>>> a compilation warning when calling free_my_item...
>>>>>
>>>>> I guess we should drop this refcount thingie, too much hassle and issues.
>>>>
>>>> I'm all for dropping it.
>>>
>>> I disagree, it's an improvement over having nothing and doing it
>>> everywhere by our own. With just this macro you have at least a common
>>> pattern that you can easily understand and track, not something
>>> depending on the dev who did the job. And more we will use
>>> Ecore_Thread, more people will need refcount and more we will have
>>> different implementation of the exact same thing.
>>
>> Here I disagree based on the following technical reasons:
>> - you're macro is not thread safe, thus not usable freely with
>> threads. You rely on specific behavior that does not touch the
>> reference count at the same time in two threads;
>
> Well, for most case, you refcount in the main loop. See current user
> of it, both refcount stuff in the main loop and unref them also in the
> main loop. That's the common case. I can add a thread safe refcount
> later, that would be another type. But in my opinion it's useless with
> Ecore_Thread worker, you do increase the refcount when you start the
> worker, not when it is running or you end up with reference somewhere
> in the pipe that are not counted. So that make it more complex.
This is not written anywhere. I know that because I followed the
development, but I'm pretty sure random eina user will do a nasty
mistake and curse you forever :-)
>> - if you make this thread-safe, then you'd waste resources on
>> common case that does not require it;
>
> Agreed.
>
>> - the code you're "abstracting" is way too simple. Maybe it's worth
>> to have some coccinele check for the reference count + free if you
>> want to detect mistakes.
>
> Yes, it is simple. And in my opinion, making it look the same
> everywhere is better than having to run a special tool to check that
> it indeed the same every where (because nobody will run that tool
> often, because every one will have to re-understand how code that are
> almost the same work).
The problem is that abstracting it under a macro/function will lead
people to consider it "magic" and always work. Particularly with
reference counting, as most (all?) other solutions work in thread
context.
Note, in some architectures you have atomic inc/dec that would remove
one problem without locks, but you still would have to test the value
later on the unref case.
>>>> Really, I already told that in the past when Andre Dieb was doing
>>>> EUPnP he asked for similar thing and I said "no, don't go that way,
>>>> it's not worth"
>>>
>>> The cost is 0, the maintenance is 0 also and it improve readability
>>> and maintenance of the code using it in my opinion. Making it an
>>> improvement.
>>
>> I don't agree with that. See above.
>
> Well, as for maintenance, you didn't say anything about that. From my
> point of view, maintenance is 0, because you don't have to do anything
> in eina anymore. And maintenance in the app is improving as you have
> the same logic for the code and you can reuse what you already know
> from a previous experience insteed of understanding how this behaviour
> was coded that time.
But consider the above. Also in some cases you don't even need
references, rather can go with some binary "in_use" flag. Evas, for
instance, worked with that for a decade :-)
Leaving these things to the user is simpler for now. Simpler than
trying to cover all cases in one. And really, I never noticed people
doing manual reference counting incorrectly in that aspect. Sure,
people forget to ref/deref, but they never do stupid things that these
simple macros try to avoid.
>>>> also, with eina_object, that I dislike the name as in Eina it's better
>>>> to just have a "memory handle" instead objects. Objects will lead
>>>> people to expect complex things like 1) inheritance; 2) interfaces; 3)
>>>> reference counting; 4) type safety (or checking). Putting all of those
>>>> in C and in making it easy-to-use is impossible... being tried over
>>>> the past 30 years without success :-P
>>>
>>> Right now, Eina_Object is limiting itself to something like
>>> inheritance (more just like C structure inheritance by packing parent
>>> on top of the structure), type safety (that was the main purpose of
>>> it) and memory repacking. I am now planning to add refcount.
>>
>> you also need some concept of class x instance. Lots of definitions
>> are easier done per-class and will avoid memory waste and useless
>> setup cycles. Similar to Evas_Smart x Evas_Object.
>
> Yes, Eina_Object have a complex of Eina_Class of course.
Hum, ok. Also thinking about this ref/unref reminded me of other
issues with it: how do you handle circular dependencies? Either you
have a GC running in a thread (bohem-gc) or you'll need 2 phase
termination like GObject (finalize x destroy). All of this smells like
more problem than solution :-/
>>> I don't like the idea of adding interfaces to it as it is where the
>>> cost come most of the time, but that's open for discussion.
>>
>> Interface is just another pointer in the type (class) definition, It
>> will contain a list of other types (kludge) or interface definitions
>> (structure with pointers + identifier).
>
> Ok, I maybe miss understand that term, but for me the interface is
> where come all the hugly function with virtual and other inheritance
> stuff. So yes, only a list of pointer with an identifier, but you need
> to provide a way to handle all the thing that people expect, and that
> have a cost that I don't see us making it better than what the other
> have done.
virtuals are already handled in our Class thing (it it looks like
Evas_Smart_Class). Interfaces are a list that are similar to this
Class, with extension points. It's a way to solve the
multiple-inheritance, to solve "this is a widget (class) that is
scrollable (interface)". Then when you call
elm_scrollable_policy_set(object), the elm_scrollable_policy_set would
be implemented like:
void ABC_scrollable_policy_set(ABC_Object *obj, int horiz, int vert) {
const ABC_Interface *iface;
const ABC_Scrollable_Iface *siface= NULL;
ABC_Class *cls;
const Eina_List *l;
CHECK_OBJECT_VALIDITY(obj);
// this should be a function on its own:
cls = GET_OBJECT_CLASS(obj);
EINA_LIST_FOREACH(cls->interfaces, l, iface)
if (iface->type = ABC_SCROLLABLE_INTERFACE)
{
siface = iface;
break;
}
if (!siface) return; // does not implement interface
// dispatch it to the interface... you could omit siface as you
can retrieve it again from obj
siface->policy_set(siface, obj, horiz, vert);
}
>>>> And to this point, if we ever have to implement a new BLA_Object
>>>> I'm more likely to be against. I'd support using existing object
>>>> systems that would get us some interpreted language for free, like
>>>> PyObject (Python) or one of the various JavaScript engines. They
>>>> already have all these things and expose it to be used in an easier
>>>> language. And don't be naive to think our would be faster, as soon as
>>>> we add everything they do, we'd be as slow.
>>>
>>> Agreed.
>>
>> Finally something we agree in this mail! :-D </joke>
>
> :-D
> --
> Cedric BAIL
>
--
Gustavo Sverzut Barbieri
http://profusion.mobi embedded systems
--------------------------------------
MSN: bar...@gm...
Skype: gsbarbieri
Mobile: +55 (19) 9225-2202
|