On Thu, 11 Jan 2018 18:25:37 -0200 Gustavo Sverzut Barbieri <barbi...@gmail.com> said:
> On Thu, Jan 11, 2018 at 5:54 AM, Carsten Haitzler <ras...@rasterman.com> > wrote: > > On Wed, 10 Jan 2018 16:52:26 -0200 Gustavo Sverzut Barbieri > ... > >> however, in a more generic OOP system this makes things unusable, as > >> we're discussing in this thread. > >> > >> parents: wisely we're advertising almost all objects should have a > >> parent, like a window, a widget, a loop... maybe it will boils down to > >> a thread/process. > >> > >> however, in these cases children is using/knowing about parents, > >> parents rarely knows/deals with children -- UNLIKE EVAS. > >> > >> --> That said, IMO parents should NOT have a reference to their children. > >> > >> if we wish to do cascade deletion, which may be very helpful (delete a > >> loop, get rid of all users... like done for evas + objects), then we > > > > that is exactly what eo is doing. it's doing "automated cascade deletion" > > from any point in a tree. if a parent goes, then the children go too (or at > > least conceptually should go). yes. in theory - you delete a loop and > > everything related to it should go away (that basically would be everything > > in that thread) ... > > there is a difference in this and the Python/Lua and the likes > mentioned before, they remove the reference, yes, but they don't lead > to resource destruction/invalidation. If there's another reference to > the object, they keep alive. this would be the same. if you do another efl_ref() or efl_xref() or efl_key_ref_set() ... they all mean there is another reference to the object just like in a gc'd lang there being another "live variable" with that obj ref init so the gc will not clean it up. same thing. it's just done manually by saying you have a ref (or don't anymore). > which is different case when the object is actually dependent on the > parent, like a timer->loop or rectangle->evas. it's the same in the case the ONLY reference to that object is in the parent which is actually the common case in most languages - a box in lua (in some hypothetical lua toolkit) would have a reference to its child objects (buttons or whatever), and that'd probably be the only reference 99% of the time... > the cascade delete I meant is "if I delete the loop, it must delete > all children, be timers OR some random class that just used loop as > parent". well it can't if there are "excess references" as i mentioned... :) these ones may be orphaned. > currently it's not, being the root cause of Andrews' question on > whether to unref/del or not... cascade delete is NOT usual in OOP, but > helps our use cases by a great deal... so may be considered a good > solution for our purpose-specific OOP that is Eo. that's the case we care about. > > though as i mentioned already - if people add extra refs to objects > > and then don't remove them when this happens some objects would get > > orphaned at this point and this is a bit of a problem. > > as I wrote in other email, keep handles alive, but invalidated. If we > follow the GObject guidelines, invalidating object (what they call > "dispose") should have it to release reference to all other objects, > but keep around essential data, like Private_Data, maybe allow some > getters to work, etc. If we opt to force cascade deletion, then the > base class should also cascade invalidate all children. yes... but even if invalidated... the object will still be around consuming an eoid (a table entry) and memory for the object... possibly forever if someone forgot about this extra ref. it may be terminated but still around. like a zombie process. zombie objects. > >> can dissociate terminate/invalidate from reference, but the last > >> reference would invalidate: > >> > >> --> NOTE: using efl_new() instead of efl_add() as I think this should > >> be renamed for clarity... > >> > >> basic usage: > >> o = efl_new(cls, parent, ...); // refcnt = 1, the caller's reference > >> efl_unref(o); // refcnt = 0, efl_invalidate(o) is auto-called > >> > >> > >> explicit invalidate: > >> o = efl_new(cls, parent, ...); // refcnt = 1, the caller's reference > >> efl_invalidate(o); // refcnt = 1 > >> efl_unref(o); // refcnt = 0, no call to efl_invalidate(o) > >> > >> canvas usage #1 (basic usage): > >> o = efl_new(EFL_UI_BUTTON_CLASS, window, ...); // refcnt = 1, the > >> caller's reference > >> efl_unref(o); // refcnt = 0, efl_invalidate(o) is auto-called, > >> button is removed from window > >> > >> canvas usage #2 (invalidate window): > >> o = efl_new(EFL_UI_BUTTON_CLASS, window, ...); // refcnt = 1, the > >> caller's reference > >> efl_invalidate(window); // o.refcnt = 1, but efl_invalidate(o) is > >> called efl_unref(o); // refcnt = 0, no call efl_invalidate(o) > >> > >> problem with this approach is that most people will forget to > >> efl_unref(o), then while it will be invalidated, its memory will leak. > > > > i thought cedric mentioned this already - efl_del would invalidate then > > unref in one call, so you don't forget. ? i dont see why we should move to > > efl_new from efl_add either. :) > > in this specific example is to make clear the reference ownership. You > called "new", you call "unref". Invalidate doesn't change that (root > of our usability issues, as pointed out by Andrew). > > the name "new" instead of "add" is to make it clear you're not adding > stuff to parent, just using it as a parent handle... it won't keep a > reference, etc... which I'd imply from something called "add". ummm... but you are adding the child to the parent. it's being added to the children list and the parent "owns" the reference here (not you the caller). > >> while this can be solved within evas legacy wrapper, the usability is > >> not the best. Other systems "solve" this with one of two options: > >> > >> - reference transference: efl_adopt_ref(receiver, o), would give your > >> reference of "o" to "receiver", which is supposed to release it on its > >> "invalidate". That is: keep an Eina_List *adopted, that is > >> automatically managed and efl_unref() on its own invalidate. > >> > >> - floating references: object starts "unowned" and the first "user" > >> should own it, for details: > >> https://developer.gnome.org/gobject/stable/gobject-The-Base-Object-Type.html#floating-ref > > > > that is how eo currently works. eo tracks the first time an object gets a > > parent. if we always start with a parent (as is central to this discussion) > > then worrying about the other code path is kind of pointless. :) > > and this is confusing! but if we "always" have a parent it's moot. > >> GObject UI and gstreamer use this "floating" and I always disliked it, > >> finding it too error prone. It "looks easy" to use, but the situation > >> is similar to what we have in EFL and led to this thread... so I'd > >> avoid it. > > > > i like it because it is easy. in c. because you have no automatic cleanup on > > exit from scope. > > > >> since unlike Gtk and GStreamer we're suggesting elements to be created > >> with a parent, as opposed to adding to a parent, this is simpler. > >> Compare: > >> > >> // GTK: > >> container = create_container (); > >> container_add_child (container, create_child()); > >> > >> // EFL: > >> container = efl_new(CONTAINER_CLS, loop, ....); > >> child = efl_new(CHILD_CLS, container, efl_container_pack(container, > >> efl_added), efl_adopt_ref(container, efl_added)); > > > > why would there need to be an explicit adopt? adding a new object with a > > parent "container" already says exactly that. the container_pack is only > > needed as an explicit "i want you to pack in the parent THIS way (at the > > end/start of a box or at specific x/y/col/rows in a table etc.). > > in this example (it's just PART of the rationale to the overall > thing), you're transferring YOUR reference to the container. That's > why. but it's defined to do exactly that. why make it so much more pain to write the code? why frustrate developers in writing extra things they just don't have to do? it's one more thing to forget and then go wrong. if every use case that people will encounter in the wild will be "add to parent and parent owns the object and will delete it when the parent gets deleted" ... how is this confusing? it's a very simple rule. it's consistent and universal and it requires less work than writing a whole bunch of "adopt" code that is already defined to be the case. > >> this is explicit, clear what's happening. Since "efl_adopt_ref()" was > >> written explicitly there, or called afterwards, it's easy to spot you > >> don't own the reference anymore. Maybe coccinelle, coverity and the > >> likes can told that rule to spot errors. > > > > but parent obj already does this... it just makes everything so pointlessly > > verbose. the add with a parent is telling the parent to take ownership of > > the object. the handle you are returned is for usability so you can call > > methods on it (outside of the add scope). just what you need for code like: > > > > o = efl_add(CLASS, parent); > > if (x) efl_whatever_x(o); > > else efl_thing_y(o); > > > > yes. technically it can be put inside the add() but eventually you'll want > > something complex enough where it just doesn't look right there. > > > >> others may claim efl_adopt_ref() is always the case, in that case > >> keeping efl_invalidate() UNRELATED to efl_unref() is also helpful: > > > > i think they are unrelated except that if invalidation is a standard part of > > the destruction cycle then it must be called when refs go to 0 even if > > that's thanks to an unref. > > > >> --> NOTE: using efl_add(), since here parents keep the reference > >> > >> WRONG/BUG usage: > >> o = efl_add(cls, parent, ...); // refcnt = 1, the PARENT's reference > >> efl_unref(o); // refcnt = 0, but parent held the reference > >> > >> basic usage: > >> o = efl_add(cls, parent, ...); // refcnt = 1, the PARENT's reference > >> efl_invalidate(o); // refcnt = 0, since parent will unref based on > >> invalidate event > >> > >> cascade delete usage: > >> o = efl_add(cls, parent, ...); // refcnt = 1, the PARENT's reference > >> efl_invalidate(parent); // refcnt = 0, cascade invalidate, leading > >> to parents unref children. > >> > >> ref usage (when you can't monitor EFL_EVENT_INVALIDATE or weak-ref): > >> o = efl_add(cls, parent, ...); // refcnt = 1, the PARENT's reference > >> efl_ref(o); // refcnt = 2, PARENT + caller > >> efl_invalidate(o); // refcnt = 1, since parent will unref based on > >> invalidate event > >> efl_unref(o); // refcnt = 0 > >> > >> > >> as a last note to this long email, shall we opt to use the "parent > >> keeps a ref" method, since we use the eoid abstraction, I'd highly > >> recommend that efl_ref() creates ANOTHER eoid for the same object, > >> with associated WARN_UNUSED_RESULT/MALLOC and efl_unref() behaves like > >> free(), considering the handle to be destroyed. This should help > >> finding bugs... it's worth since efl_ref() will be rarely used. > > > > oh no no. efl_ref() is lightweight and slim. creating more eoid's is going > > to be super heavy in comparison. > > point is: you shouldn't need efl_ref() or efl_unref() if the ownership > is clear. Currently it isn't, thus we need to safeguard with things eh? if i have an object A that is a child of B, and i also do list = eina_list_append(mylist, A); i want to ALSO do efl_ref(A); because i have it in my list (unless i listen to the del event etc. and track it like a weak ref and remove from the list). if i have an async event that needs to hold onto the original object until its done, then you have to add a ref. it's cheap and simple. just ref++ and ref-- on unref internally. it's an incredibly common pattern and i don't see that it should be made heavier than it is. eoid's are not a limitless resource either. on 32bit there are 2 million of them available. reference counting is one of the most common things i've seen for object lifetime management in addition to gc's so what's bad with what we have? if its a new eoid ... and the original obj is somehow unreffed to 0 then you have an error when eoid 1 tries to find eoid 2. the same error you'll have if its the same eoid and the original is deleted and now another unref happens to a now invalid eoid ... it'll boil down to the same just with different eoid's instead of the same one, with added pressure on the eoid space and extra overhead in memory, and cost to ref and unref. > such as efl_ref() and efl_unref() when calling back the user from > non-eo methods (ie: callback from other object), as suddenly the > object may go away. At least in cases where I'm dealing with children > objects, I'd own reference to them automatically, they can never be > deleted (or unparented) letting me get a deleted object when I return > to my context. > > also, this would help with your quest to spot incorrect usages, you > couldn't unref more than once the same id, it would complain... and > would be easy to track leaks, since you could associate efl_ref() with > the calling backtrace. you could. it'd just become an invalid eoid reference no matter what happened. it'd indeed make it easier to spot precisely WHERE the extra ref or unref comes from as it'd have its own unique eoid to track, but despite that i like the extra info i heavily dislike the cost. > > -- > Gustavo Sverzut Barbieri > -------------------------------------- > Mobile: +55 (16) 99354-9890 > -- ------------- Codito, ergo sum - "I code, therefore I am" -------------- Carsten Haitzler - ras...@rasterman.com ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
