Hi,
I created a new event "destruct" in Efl.Object.
It's triggered just before removing the callbacks, in the base class
destructor, as it's both easier and safer this way. I guess this can be
used in bindings too.
As for the rest, I need to think about it more.
Best regards,
On Wed, Jan 10, 2018 at 1:50 AM, Carsten Haitzler <ras...@rasterman.com>
wrote:
> On Tue, 9 Jan 2018 21:03:53 +0900 Jean-Philippe André <j...@videolan.org>
> said:
>
> > On Tue, Jan 9, 2018 at 3:17 PM, Carsten Haitzler <ras...@rasterman.com>
> > wrote:
> >
> > > On Mon, 8 Jan 2018 18:15:13 +0900 Jean-Philippe André <
> j...@videolan.org>
> > > said:
> > >
> > > > Hi,
> > > >
> > > >
> > > > Some of the argumentation here seems to lack a bit of a scientific
> > > > approach...
> > > > So, I looked for all occurrences of efl_add() (see PS for
> methodology).
> > > >
> > > > In all of EFL, without specific filters:
> > > >
> > > > Overall: 244 unique classes / 1572 occurrences
> > > > No parent: 119 unique classes / 472 occurrences
> > > > 48% of classes used without parent, accounting for 30% of all uses.
> > >
> > > i argue that the vast majority of those no parent cases should have a
> > > parent. i
> > > actually did a grep -r through efl too looking and almost all the cases
> > > with a
> > > null parent i noted were bugs and should be fixed. :) i specifically
> noted
> > > some
> > > of those examples (the efl_anim stuff for example - i filed a bug for
> it
> > > too).
> > > so while i didn't do numbers... i did do an analysis. quickly in about
> 2
> > > minutes. for a more detailed analysis. note when i say "loop should be
> > > parent"
> > > i mean loop directly OR some object that ultimately has a parent that
> is
> > > loop
> > > at the top of the object tree:
> > >
> > > src/benchmarks/eo/eo_bench_* (11)
> > > just benchmarking so not real code. (ignore)
> > > src/bin/elementary/test_*.c (86)
> > > all are bugs. every one should have loop as parent.
> > > src/examples/ecore/ecore_audio*.c: (7)
> > > all are bugs. loop should be parent (loop drives i/o for audio and
> cb's)
> > > src/examples/ecore/ecore_idler_example.c: (1)
> > > bug. loop as parent (commented out though)
> > > src/examples/ecore/ecore_poller_*.c (3)
> > > bug. loop should be parent
> > > src/examples/ecore/efl_io_*.c (8)
> > > bug. loop should be parent
> > > src/examples/ecore/efl_net_*.c (6)
> > > bug. loop should be parent
> > > src/examples/eio/eio_sentry.c (1)
> > > unknown
> > > src/examples/elementary/efl_ui_*.c (4)
> > > bug. loop should be parent
> > > src/examples/elementary/file*.c (9)
> > > bug. loop should be parent
> > > src/lib/ecore/ecore.c (1)
> > > bug. singleton vpath object will fail with multiple loops, so loop
> > > should be
> > > parent, but this is unused atm so it doesn't matter yet.
> > > src/lib/ecore/efl_loop.c (1)
> > > correct null parent usage
> > > src/lib/ecore/efl_model_composite_*.c (2)
> > > relatively sure these are bugs. in fact these files do a lot of:
> > > Efl_Promise *promise = efl_add(EFL_PROMISE_CLASS,
> efl_main_loop_get());
> > > and similar assuming only the main loop... which is wrong. they
> should
> > > get
> > > the correct loop, not always the main loop. lots of examples of this
> > > through
> > > efl.
> > > src/lib/ecore_con/ecore_con_*.c (5)
> > > bug. should use loop as parent
> > > src/lib/ector/cairo/ector_cairo_surface.c: (3)
> > > src/lib/ector/gl/ector_gl_surface.c: (3)
> > > src/lib/ector/software/ector_software_surface.c: (3) (9 total)
> > > this is actually ok since ector doesn't use any async events and
> needs
> > > no loop
> > > src/lib/edje/edje_*.c (10)
> > > bug. loop should be parent
> > > src/lib/efl/interfaces/efl_vpath_core.c: (1)
> > > bug. vpath needs to be loop driven due to the ability to do async
> lookups
> > > src/lib/efl/interfaces/efl_vpath_manager.c (1)
> > > bug. same as above
> > > src/lib/eio/eio_model.c: (1)
> > > smells like a bug. eoi would need a loop to drive it
> > > src/lib/elementary/efl_access.c: (1)
> > > bug. access relies on events and async (e.g. dbus) and so must have
> loop
> > > src/lib/elementary/efl_ui_*.c: (15)
> > > bug. need loop parent
> > > src/lib/elementary/elm_*.c (5)
> > > all look like bugs. need loop parent
> > > src/lib/evas/canvas/efl_animation*.c (7)
> > > bug. need loop parent
> > > src/lib/evas/canvas/efl_canvas_vg.c: (1)
> > > bug. need loop parent
> > >
> > > src/lib/evas/canvas/evas_main.c: (1)
> > > bug. need loop parent
> > > src/lib/evas/canvas/evas_vg_node.c: (1)
> > > bug. vg nodes should have a parent object - always
> > > src/lib/evas/gesture/efl_gesture_manager.c: (1)
> > > bug. need loop parent
> > > src/modules/evas/engines/gl_generic/evas_engine.c (3)
> > > src/modules/evas/engines/software_generic/evas_engine.c: (3) (6 total)
> > > correct. ector objects
> > > src/tests/ecore/ecore_test_ecore_audio.c (16)
> > > bug. need loop parent
> > > src/tests/ecore/ecore_test_promise2.c: (2)
> > > bug. need loop parent
> > > src/tests/ecore_con/ecore_con_test_efl_net_ip_address.c: (14)
> > > bug. need loop parent
> > > src/tests/efl/efl_test_model_*.c: (5)
> > > bug. need loop parent
> > > src/tests/efl_js/benchmark_object_impl.cc: (1)
> > > artificial benchmarking (so ignore)
> > > src/tests/efl_mono/libefl_mono_native_test.c: (1)
> > > artificial benchmarking (ignore)
> > > src/tests/eina_cxx/eina_cxx_test_*.cc (54)
> > > artificial testing of basic input/output through eo api
> > > src/tests/eio/eio_test_sentry.c: (20)
> > > bug. should have parent loop
> > > src/tests/eldbus/eldbus_test_*.c: (3)
> > > bug. should have loop parent
> > > src/tests/elementary/elm_test_*.c: (31)
> > > bug. should have loop parent
> > > src/tests/eo/access/access_main.c: (1)
> > > bug. should have loop parent
> > > src/tests/eo/children/children_main.c: (1)
> > > artificial eo testing - ignore
> > > src/tests/eo/composite_objects/composite_objects_main.c: (1)
> > > artificial eo testing - ignore
> > > src/tests/eo/constructors/constructors_main.c: (1)
> > > artificial testing - ignore
> > > src/tests/eo/function_overrides/function_overrides_main.c: (4)
> > > artificial testing - ignore
> > > src/tests/eo/interface/interface_main.c: (1)
> > > artificial testing - ignore
> > > src/tests/eo/mixin/mixin_main.c: (1)
> > > artificial testing - ignore
> > > src/tests/eo/signals/signals_main.c: (1)
> > > artificial testing - ignore
> > > src/tests/eo/suite/eo_test_*.c: (94)
> > > artificial test suite - ignore
> > > src/tests/eolian_js/eolian_js_test_constructor_method_impl.c: (2)
> > > artificial test suite (ignore)
> > >
> > > so i spent a lot of time going over every instance (well with a git
> grep
> > > for
> > > eo_add( and NULL on the same line).
> > >
> > >
> > > 459 instances of an eo_add with NULL. i get 1572 eo_add's with a git
> grep
> > > ...
> > > uses. my numbers differ from yours, but ok. we're doing rough stats
> here.
> > >
> > > 173 instances are artificial testing so let's remove them from our
> stats.
> > >
> > > 286 eo_adds with null that are "real". 1399 total adds when we remove
> the
> > > artificial testing ones. of those 286 instances, 1 is "unknown" if it's
> > > correct. let's remove that from our stats for now. 285 null adds, 1398
> > > total
> > > eo_add's. of the 285 null adds.. only 16 are correct uses with null
> > > parent. the
> > > rest are bugs. so ... 16/1398 are valid uses of null parent adds.
> that's
> > > 1.1%
> > > of use cases.
> > >
> > > but wait... of those 16 "null parent is ok" examples... 1 (the main
> loop
> > > add)
> > > is internal to efl only and never seen or used outside. loops are and
> will
> > > be
> > > created for you. so it's actually 15. 15/1398 is almost exactly 1%.
> > >
> > > my "spitballing rough number from a glance at git grep and where the
> calls
> > > were" was almost EXACTLY spot on. so i totally stand by my 1-2% number
> i
> > > roughly
> > > guessed in a 1 minute scan. of course now you forced me to spend an
> hour
> > > doing
> > > the above.
> > >
> > > now an accurate evaluation says it's 1%. and that's WITHIN efl. the 1
> case
> > > of
> > > the main loop of the 16 will not exist outside of efl itself as loops
> will
> > > be
> > > created for you, and the other 15 are all ector.
> > >
> > > imho ector is a low level api for doing raw rendering and describing a
> > > display
> > > graph. in fact for sheer performance reasons as long as it's kept
> internal
> > > to
> > > efl it shouldn't be an eo api for speed reasons.
> > >
> > > will we ever expose a "direct immediate mode rendering api" to users?
> like
> > > ector? especially when we have evas vg objects that hide this? and in
> the
> > > event
> > > you used such an object... and if we exposed ector objects as the
> actual
> > > "vector tree/graph" ... they will then all have a parent - the vg
> object
> > > containing them.
> > >
> > > so i actually argue that the 16 cases i counted are actually 0. either
> > > ector
> > > shouldn't be eo and stay a low level api internally to efl, or we
> expose
> > > ector
> > > node graph within a vg object and then they all have parents.
> > >
> > > so after some analysis of the above... no a single "external use" of
> > > efl_add
> > > should be done with a null parent. and the only internal "valid" use
> is the
> > > efl_add of the loop objects. ector is kind of an odd/exception state
> atm as
> > > above.
> > >
> >
> > Well if the thinking is "all objects should have a parent" then of course
> > exactly 0% of them will have no parent. Basically only a "thread" object
> > would be allowed to be parentless. Windows should then be created with a
> > loop as parent, internally reparented to evas if needed.
>
> that is exactly my thinking and it's where i'm coming from. i thought i
> made
> that clear in prior emails that i think this is where it's boiling down to
> -
> that i see efl as a "basically almost no objects have NULL parents, so
> what's
> the fuss all about? what we need to do is police parents and ensure they
> are
> correct (that the parent is a mainloop object where it must be main loop
> only
> or is non-null in almost all other cases)". it's a policing issue not an
> api
> issue.
>
> > If we take this approach then @noparent is not even required (only one
> > special kind of object should have no parent, for which we could have an
> > internal api).
> > This could solve inconsistencies.
>
> well we'd have to hide the ability to efl_add that class outside of efl
> itself.
> for now just documenting that all objects must have a parent, then moving
> to
> enforcing it (well first fixing all the bugs we know about then fixing
> them to
> have the correct parent so it properly works with multi-loop/thread later
> then
> putting in policing/enforcement code in eo... or something like this). but
> going in this direction i think is the way to go. we can have some
> exceptions
> for now that don't need a parent (ector and loop objects - loop objects
> can be
> documented with "never create one of these yourself"... for now).
>
> > > In all of EFL, without EFL_UI_WIN_CLASS or ECORE_XXX:
> > > >
> > > > Overall: 226 unique classes / 1438 occurrences
> > > > No parent: 89 unique classes / 358 occurrences
> > > > 42% of classes used without parent, accounting for 27% of all uses.
> > > >
> > > >
> > > > Only in src/examples, without EFL_UI_WIN_CLASS or ECORE:
> > > >
> > > > Overall: 44 unique classes / 474 occurrences
> > > > No parent: 12 unique classes / 27 occurrences
> > > > 27% of classes used without parent, accounting for 5.7% of all uses.
> > > > (note: only one example with EFL_UI_WIN_CLASS)
> > > >
> > > >
> > > > In src/bin/elementary, without EFL_UI_WIN_CLASS or ECORE:
> > > >
> > > > Overall: 49 unique classes / 303 occurrences
> > > > No parent: 14 unique classes / 54 occurrences
> > > > 29% of classes used without parent, accounting for 18% of all uses.
> > > > (note: those classes are animation and interpolator)
> > > > (note2: 34 occurences of WIN class - no parent)
> > > >
> > > >
> > > > Conclusions:
> > > >
> > > > 0. This analysis is flawed by design due to a lack of real usage.
> > >
> > > correct. the above numbers include lots of code that needs to not use
> null
> > > parents. my more detailed analysis shows my numbers of 1-5% to be
> > > incredibly
> > > generous... in current efl it's actually 0%. every existing instance
> of an
> > > efl
> > > add with null inside efl or in tests is a bug (excepting the above as i
> > > described).
> > >
> > > > 1. Those numbers differ from the values given in the below email.
> > > > While I understand where this "maybe 1 or 2% of all objects [are
> created
> > > > without a parent]" comes from, it's not based on facts.
> > >
> > > actually it is... because almost all the instances of adding with null
> > > parents
> > > are bugs atm. even many without null parents are currently bugs (they
> use
> > > the
> > > wrong parent - eg always get the mainloop singleton instead of use the
> > > correct
> > > loop parent of that object like the model examples i pointed to).
> > >
> > > > @noparent makes sense. A NULL check in efl_add would then help.
> > >
> > > that is my compromise. or what i am thinking is a good compromise.
> that in
> > > the
> > > very very very very few exceptions where we allow null parents... we
> > > specifically tag it.
> > >
> >
> > Might as well have no exception, except internally for a special TLS
> > "thread" object that would be the root object to rule them all.
>
> actually my thoughts are the loop object is that root. when you create a
> new
> thread you get a thread object on the creator side, and then the thread
> itself
> starts with a loop object like EFL_MAIN does with an args callback. so the
> loop
> object within that thread is also thread-local ... and the loop
> communicates
> back to the thread object on the "creator/parent" side e.g. via pipes.
>
> > > A different API then wouldn't hurt either IMHO (maybe efl_new?
> > > > efl_add_single? efl_create? or whatever -- efl_add then can NOT be
> called
> > > > with NULL).
> > >
> > > once the bugs are fixed... at this point we'll be create an api though
> with
> > > ZERO external use outside of efl and only 16 instances within efl
> itself.
> > >
> >
> > There were no "bugs" until it somehow was decided that all objects should
> > have a parent.
>
> i thought we talked about this long enough - that objects will be driven by
> their loop and the loop will be a parent so they know what they are driven
> by.
> we cant have multiple loops unless we do this because then objects have no
> idea
> what loop will drive their async i/o at all.
>
> > as i said. we should be policing the null parents because basically 99%
> of
> > > efl
> > > objects (or instances of their adds) do not allow null parents.. or
> SHOULD
> > > not
> > > once we've fixed everything up.
> > >
> > > > 2. The argumentation in this email chain again leads nowhere. The
> > > original
> > > > confusion remains mostly unaddressed.
> > >
> > > i think not. i made the argument that it's not really confusing - it
> > > follows a
> > > simple rule... BUT that reality is in no real life cases will anyone
> using
> > > the
> > > efl api be adding objects with a null parent. it's a "0% of use cases"
> > > thing.
> > > so as there is no valid instance of doing an add with a null parent...
> why
> > > do
> > > we have to make things more confusing by even having an api that
> > > encourages it?
> > >
> > > > Felipe mentioned that ownership and references are mixed. So the
> proposal
> > > > for efl_release (or detach, close, invalidate, ...) makes sense to
> me.
> > > > In fact we have issues sometimes with efl_del as inside a destructor
> we
> > > > already lost our parent (thus all efl_provider_find and related calls
> > > will
> > > > fail).
> > >
> > > i brought up this issue specifically already in this and related
> threads
> > > and
> > > said it's horrible and painful and unparenting should be done outside
> the
> > > the
> > > constructor (after last constructor called). not having a parent when
> > >
> >
> > "destructor (after last destructor called)" -- I guess?
>
> sure. but we kind of agree here. :)
>
> > > destructor is called is leading to some major ugliness in objects
> having to
> > > manually track their loop so they at least know what loop they did
> belong
> > > to
> > > during destruction so they can clean up some internals like "fd
> handlers"
> > > etc.
> >
> > yup
>
> seems we agree. :)
>
> > > > Also, quite a few times I've also been looking for a "deleted" event
> that
> > > > would happen after destruction, and not before.
> > >
> > > well there is the case for a del that is called just before
> destructors are
> > > called so everything in the object is still there. that's what we have
> now
> > > (or
> > > should have). there might be the case for another callback to be
> called in
> > > the
> > > finally destructor just when all the event callbacks are about to be
> > > cleaned
> > > out. you couldnt call something after the actual final destruction
> though
> > > as
> > > the object now no longer exists... :)
> >
> > > I had to introduce a very ugly API very badly called
> "allow_parent_unref"
> > > > in efl.object because some objects need a parent but they should be
> > > > unref'ed by someone else (efl_part objects but not only).
> > > >
> > > > So, I think it would make sense to investigate efl_terminate, and
> > > > evas_object_del would just call efl_terminate, hiding the object or
> > > > starting destruction, then let the parent (either evas or another
> canvas
> > > > object) do the final unref/unparent and destroying everything that's
> > > left.
> > > >
> > > >
> > > > Last note: @owned is rarely used with objects:
> > > > - Efl.Net.Ip_Address create (factory pattern, replaces efl_add, but I
> > > think
> > > > if there's a parent then we shouldn't tag as @owned)
> > > > - efl_duplicate (creates a new object like efl_add)
> > > >
> > > >
> > > > Anyway I think experiments only can tell us what's best. I see 3
> items:
> > > > - @noparent tag
> > >
> > > that i agreed with... and there are almost no instances that would use
> it.
> > > loop
> > > is one. in fact it should have a "never call efl_add on me - ever". ...
> > > unless
> > > we want to get into the complexity of child and parent loops within one
> > > thread.
> > > i really dislike that complexity and would at this stage want to just
> ban
> > > it
> > > entirely thus no one ever should efl_add a loop. ... except inside efl
> ...
> > >
> > > > - efl_new (= efl_add without a parent -- requires @noparent if we
> want
> > > > strong NULL check)
> > >
> > > right now we have no "public" instances of this being allowed. in
> future i
> > > am
> > > imagining possibly shared objects that are transported from thread to
> > > thread
> > > being able to be like this (that's my 1-2% number). even then they may
> > > still
> > > have some global parent "app" object they belong to. and that app
> object
> > > would
> > > be "created for you". so even then... i don't see a need to allow null
> > > parents
> > > (except the app object itself which will like loop, be created for
> you).
> > >
> > > > - efl_invalidate / efl_terminate (I prefer efl_close :P)
> > >
> > > i actually prefer close too. shorter.
> >
> > But as cedric said there is file close (and specifically
> > efl.io.closer.close). Assuming this "invalidate" is a method (declared in
> > .eo, available in bindings), its name shouldn't conflict :)
> >
> > Anyway this opens some questions:
> > - do we really want to have parents for all objects? (basically except
> for
> > loop or app or some thread root object)
>
> without this hjow can an object do any async i/o - how can it register
> fd's or
> timers/timeouts, jobs, handle animation or just about anything i/o related.
> having a parent means you know which loop to look at to add these things.
> without that you don't know. yes - ui will only allow 1 loop to be the
> parent,
> but in the general sens, for elf.net, efl.model and so much mucre - they
> need a
> loop to drive them and that would necessitate a loop being a parent
> (somewhere
> in the tree so it's findable)..
>
> > - should we even go as far as forbidding null parent? or do we still want
> > to have this exceptional case?
>
> i think in 99% of cases - yes. it's just a few cases where we would allow
> it
> and only for internal purposes (otherwise how do we create the toplevel
> loop
> object then at all?). for now just documenting all objects need a parent
> is a
> good step and ensuring our test and sample and internal code follows that.
> this
> is exactly why i know i had to do this efl loop work to have the ability
> to do
> multiple ones so it'd bring out this issue and require it to be fixed.
> it's a
> baseline assumption for eo/efl going forward.
>
> we could have eo eventually enforce this, and maybe have a backdoor to
> disable
> it for only the exceptions we are talking about. for now i think we just
> need
> to document and clean up the code we have. enforcing can be done later.
>
> > - then what happens when parent_set(NULL) is called? should this call
> > invalidate automatically?
>
> imho it should either be an error (not allowed) ... or it should
> unref+invalidate.
>
> > Or am I going too far here and we should just try to have parents on all
> > objects for our API as a policy only (objects declared outside of EFL
> would
> > then chose if they need a parent or not)
>
> for now i think a policy will be good. it'll be one of those "1+1 is 2"
> rules
> - basic things you need to know when dealing with efl. all objects must
> have a
> parent. that parent will trace back to a loop object and loop objects
> "drive"
> timeline, async i/o and events etc.
>
> eventually i think:
>
> 1. efl_add() with a null parent will fail and return null EXCEPT for the
> excepted classes (2 of them only i think in the end. definitely the loop
> class
> and maybe the "process" class that is a shared object covering the entire
> process and thus will be auto-locked and threadsafe for you. this process
> class
> i think is still up for debate, but i see it as a useful shared data store
> for
> a process as a whole).
> 2. like #1 - efl_parent_set to NULL will result in an unref+terminate
>
> one question is:
>
> if a child object has more refs than the parent... when the parent is
> destroyed... what happens to the child? i mean ... it must logically be
> unparented... but then it's floating... :( this is not pretty. that's why i
> think parent set to null does a terminate and unref so the object goes
> into a
> terminated state and is floating only because someone added more refs to
> it.
> this still has issues where the destructor now doesn't know what loop it
> belongs to... :(
>
> > //The following thoughts assume we have parents on ALL objects (except
> app
> > magic obj).//
>
> btw - i think we're on the same page here now where my point of view has
> been
> coming from the above (that we have an object tree where every object
> traces
> back to some parent than owns it with very few exceptions, and that the
> vast
> majority must trace back to a loop object out of the necessity to have a
> loop
> driving async i/o etc.).
>
> > So, to summarize the idea:
> >
> > - efl_add() -> always with a parent
>
> ultimately - yes. initially just by policy for efl everything will/must
> have a
> parent (with the few exceptions as discussed - loops, maybe process class
> and
> ector objects).
>
> > - efl_invalidate() -> ??? does not unref
>
> agree
>
> > - efl_parent_set(null) -> the parent calls invalidate, then unrefs which
> > calls the destructor and then actually set parent to null
>
> agree
>
> > - efl_del() -> always parent_set(null), no if
>
> hmm i think efl_del is just inavlidate + unref? or just an alias for unref?
>
> > - efl_unref() -> only to be used after efl_ref()
> > - efl_unref() to 0 ref -> error case as it is now, destroys the object
> but
> > prints an ERR (bindings should be safe). normal for efl_part().
>
> imho this should not be an error.
>
> > - efl_add_ref() -> only to be used by bindings, always with a parent,
> > always has 2 refs. requires implicit (by binding) efl_unref(). if used in
> > C, requires both efl_del and efl_unref to delete, unref only if we leave
> it
> > to the parent
>
> agreed... with the addition of "OR if you want to use this api like a
> gc'd/scope referenced language and you WILL do all the unrefs on scope exit
> yourself and get it right 100% guaranteed 0 but your job, so only use this
> if
> you really are sure."
>
> > - efl_destructor() -> called with a parent, but zero ref.
>
> agree
>
> > - efl_part() -> no change (returns an auto-deleted object except in
> > bindings -- as implemented for C++ already)
>
> agree
>
> > what exactly should happen in invalidate? for evas object just hide and
> > mark as delete_me?
>
> yeah. files close(), maybe any async actions like futures/promises get
> cancelled
> etc.?
>
> > one can then wonder if the parent ref is necessary: no ref + no parent =
> > deleted, no ref but parent = still alive.
>
> well strictly the parent -> child relationship ... is a reference. :)
>
> > - "invalidate" event is called when invalidate begins, "del" event is
> > emitted when destruction begins, "destroyed" event emitted after it's all
> > done
>
> yes, yes, and i prefer "deleted" rather than "destroyed" ... :)
>
> > Sorry... I think I'm still tired from jetlag... So please correct me if
> I'm
> > wrong.
> > But I am worried of making any radical change at this point in time... So
> > let's make sure we have something solid that we're all happy about. The
> > less changes the better.
>
> well i think the loop as parent is a change that is a long time coming
> that we
> all know has been needed for reasons above, but we've just glossed over it
> and
> ignored it etc. ... :)
>
> > Best regards,
> >
> >
> > >
> > > > Best regards,
> > > >
> > > >
> > > > On Fri, Jan 5, 2018 at 2:43 PM, Carsten Haitzler <
> ras...@rasterman.com>
> > > > wrote:
> > > >
> > > > > On Thu, 04 Jan 2018 17:18:35 +0000 Andrew Williams <
> > > a...@andywilliams.me>
> > > > > said:
> > > > >
> > > > > > Hi,
> > > > > >
> > > > > > I don't think I can do this any longer. Any comment I make about
> > > > > usability
> > > > > > is met with
> > > > > >
> > > > > > "it's easy to understand if you know the internal
> state/functioning
> > > of
> > > > > the
> > > > > > object".
> > > > >
> > > > > believe what you want... i described it in a single if statement.
> it
> > > > > requires
> > > > > no knowledge of internal state. it requires knowing just this:
> > > > >
> > > > > if (parent) { parent now responsible for unreffing }
> > > > > else { you (the caller) are responsible for unreffing when done }
> > > > >
> > > > > requires nothing more than that. why do you claim you need to know
> > > internal
> > > > > state? the above if has nothing to do with internal state. it has
> > > TOTALLy
> > > > > to do
> > > > > with externally provided params. it's the same logic as (assuming
> > > there is
> > > > > an
> > > > > efl_child_add that has a parent param that must be non-null, and
> > > efl_add
> > > > > now
> > > > > drops parent param as it's assumed to be NULL):
> > > > >
> > > > > if (used efl_child_add) { parent now responsible for unreffing }
> > > > > else (used efl_add) { you (the caller) are responsible for
> unreffing
> > > when
> > > > > done }
> > > > >
> > > > > both are conditions/data provided by the caller. one is a choice by
> > > > > parameter
> > > > > and the other is a choice by function call name.
> > > > >
> > > > > > My premise is still
> > > > > >
> > > > > > "with efl_add sometimes returning ownership and other times not
> this
> > > is
> > > > > > confusing for the developer"
> > > > >
> > > > > totally based on input provided by the caller. examples:
> > > > >
> > > > > fopen("filename", "r");
> > > > > vs
> > > > > fopen("filename", "w");
> > > > >
> > > > > with one i can only call fread() on the result anxd the other i can
> > > only
> > > > > call
> > > > > fwrite(). by your logic this is highly confusing to developers and
> libc
> > > > > should
> > > > > have:
> > > > >
> > > > > fopen_read("filename");
> > > > > +
> > > > > fopen_write("filename");
> > > > >
> > > > > because developers would otherwise be confused.
> > > > >
> > > > > > I wanted for us to present something that is clear and simple
> without
> > > > > > needing to know internal state or reading the EFL source code.
> > > > > > It seems I will not succeed.
> > > > >
> > > > > my argument is that more constructors make for something LESS
> simple.
> > > > > while it
> > > > > removes the parameter you do not like, it means you have to choose
> > > which
> > > > > constructor to use based on situation and know about both.
> > > > >
> > > > > also i argue that 95%+ of uses will be with pent not allowed to be
> > > NULL.
> > > > > efl.net is reliant on a loop to drive it. all of the ui is too.
> the
> > > loop
> > > > > and
> > > > > its related constructs are too. ecore_audio is too. well it is
> reliant
> > > on
> > > > > the
> > > > > main loop to drive it but it may not actually technically rely on
> > > parent
> > > > > object
> > > > > (yet, but it should). eio is reliant on parent objects too. ledbus
> too.
> > > > >
> > > > > in fact a quick look at out current efl interfaces means EVERY
> SINGLE
> > > > > object we
> > > > > have requires a parent EXCEPT the loop object ... and that is
> created
> > > for
> > > > > you
> > > > > (and will be in future with threads and their loops). so in fact
> using
> > > a
> > > > > non-NULL parent is in fact invalid in 100% of efl use cases. this
> is
> > > why i
> > > > > keep
> > > > > pushing back strongly. your argument for confusion is actually
> > > covering 0
> > > > > use
> > > > > cases (and the only reason objects can be created with NULL parents
> > > now is
> > > > > that
> > > > > we aren't policing or enforcing parents at this point and we SHOULD
> > > be. we
> > > > > SHOULD have a specific "@noparent" tag for classes that indicate an
> > > object
> > > > > is
> > > > > allowed to have no parent - it'll be by far the exception and not
> the
> > > > > rule...
> > > > > by exception i mean maybe 1 or 2% of all objects). does this
> incredibly
> > > > > rare
> > > > > case justify multiplying the number of constructors we have and
> making
> > > a
> > > > > longer
> > > > > winded one (efl_child_add) which will be actually 99% of use cases?
> > > > >
> > > > > i think we're at a position where we see 2 totally different
> things. i
> > > see
> > > > > an
> > > > > efl api where parent is basically always used. you don't. you
> believe
> > > > > it'll be
> > > > > "random" or basically sometimes NULL, sometimes not.
> > > > >
> > > > > as i already mentioned in another mail. we're missing the policing
> of
> > > > > this. we
> > > > > should actually have eo require a non-null parent for essentially
> every
> > > > > object
> > > > > we have with some exceptions (like loop objects... which will be
> > > created
> > > > > for
> > > > > you buy efl itself anyway - but in this case the check, when it
> finally
> > > > > exists,
> > > > > should be disabled).
> > > > >
> > > > > at this point in time, it'd be bad to put in a forced non-null
> check
> > > > > because
> > > > > we're still mis-using efl's api a lot and need to clean this up so
> > > it's a
> > > > > very
> > > > > small number of instances to fix before enforcing. we could add ERR
> > > logs to
> > > > > non-null parents and add a lot of noise at this point... but IMHO
> this
> > > is
> > > > > what
> > > > > we should do.
> > > > >
> > > > > now what do we do if you, by accident, pass in a null parent on
> add? as
> > > > > long as
> > > > > it's not one of the "i allow null parents" object types (which
> frankly
> > > > > will be
> > > > > a handful in the end), we probably should not even begin
> construction
> > > and
> > > > > error
> > > > > log and return NULL as the object as it's invalid. but first we
> need
> > > the
> > > > > ability to flag objects as allowing null parents and then begin
> > > cleaning
> > > > > up our
> > > > > code to match.
> > > > >
> > > > > > Andy
> > > > > >
> > > > > > On Thu, 4 Jan 2018 at 06:32 Carsten Haitzler <
> ras...@rasterman.com>
> > > > > wrote:
> > > > > >
> > > > > > > On Wed, 03 Jan 2018 09:43:16 +0000 Andrew Williams <
> > > > > a...@andywilliams.me>
> > > > > > > said:
> > > > > > >
> > > > > > > > Hi Cedric,
> > > > > > > >
> > > > > > > > I think I agree, if we can tidy the definitions then
> everything
> > > > > should
> > > > > > > get
> > > > > > > > clearer.
> > > > > > > >
> > > > > > > > efl_add_ref being needed for bindings implies that our
> > > definition of
> > > > > > > > efl_add was not clean enough in the first place.
> > > > > > >
> > > > > > > we were very clear on that. for bindings it's needed (or if you
> > > want to
> > > > > > > write
> > > > > > > code in that style) but it's highly inconvenient and we've been
> > > over
> > > > > that.
> > > > > > >
> > > > > > > > If efl_del is "hide and unparent" then maybe what we really
> need
> > > is
> > > > > > > > efl_gfx_hide and efl_parent_unset - but I don't see why we
> need
> > > to
> > > > > > > unparent
> > > > > > > > anyhow...
> > > > > > >
> > > > > > > well if it's the last ref... the unparenting cleanly removes
> from
> > > the
> > > > > > > parent
> > > > > > > AND removes the last ref as a result.
> > > > > > >
> > > > > > > BUT i kind of agree with you here. the unparenting is proving
> to
> > > be a
> > > > > major
> > > > > > > pain in the rear. by that i mean the unparenting happens and
> then
> > > > > parent is
> > > > > > > NULL and THEN as a result of this reference goes to 0 and
> > > destructors
> > > > > get
> > > > > > > called. when the destructors are called, the parent is already
> > > NULL and
> > > > > > > this
> > > > > > > has proven an "odd" case i've been dealing with the efl loop
> > > work...
> > > > > > > because on
> > > > > > > destruction the object doesn't know what loop it belonged to
> > > anymore...
> > > > > > > yes -
> > > > > > > you have to handle parent_set to NULL then but it means this
> has
> > > to do
> > > > > some
> > > > > > > kind of destruction of loop bound resources... :(
> > > > > > >
> > > > > > > IMHO when the destructors are called the object should still
> have a
> > > > > parent
> > > > > > > and
> > > > > > > be removed from the child list as a very last "after last
> > > destructor is
> > > > > > > called"
> > > > > > > step, not "before destructors are called".
> > > > > > >
> > > > > > > also we need to do better parent policing.
> > > > > > >
> > > > > > > 1. objects that can never have a NULL parent need to be marked
> as
> > > such
> > > > > > > 1.2 objects that MUST have a loop parent at the top of their
> parent
> > > > > tree or
> > > > > > > somewhere in it (provider_find for a loop class must fund a
> > > non-null
> > > > > loop
> > > > > > > object).
> > > > > > > 1.3 objects MUST have a loop object and it MUST be the main
> loop
> > > > > object and
> > > > > > > only that one.
> > > > > > > 2. some objects are intended to be toplevels (with NULL as the
> > > parent)
> > > > > and
> > > > > > > should be marked as such (e.g. loop objects).
> > > > > > >
> > > > > > > > If we are delegating to a parent to manage the lifecycle of
> the
> > > > > object
> > > > > > > then
> > > > > > > > we should step away from the reference and forget it - that
> is
> > > the
> > > > > most
> > > > > > > > "convenient" behaviour I guess.
> > > > > > > >
> > > > > > > > if:
> > > > > > > > efl_add *always* returned an owned reference and took no
> parent
> > > > > > > > efl_add_child *never* returned an owned reference and
> required a
> > > > > parent
> > > > > > > >
> > > > > > > > then:
> > > > > > > > efl_add_ref* would no longer be required right? (if the
> binding
> > > > > requires
> > > > > > > a
> > > > > > > > ref after efl_add_child we have efl_ref that it could wrap
> up)
> > > > > > > > efl_del would take a reference and dec (probably not needed
> as we
> > > > > have
> > > > > > > > efl_unref?)
> > > > > > > > efl_del_child seems unlikely to be needed as all that is
> left is
> > > > > hiding
> > > > > > > > which is a graphics call, not an eo lifecycle one.
> > > > > > > >
> > > > > > > > The more I look at it the more I think we have too much UI
> > > related
> > > > > > > thinking
> > > > > > > > in our object lifecycle.
> > > > > > >
> > > > > > > that's because 90% of our objects have been UI related in the
> past
> > > sand
> > > > > > > pretty
> > > > > > > much still are. for us, i think this is the right thing.
> > > > > > >
> > > > > > > > Andy
> > > > > > > >
> > > > > > > > On Wed, 3 Jan 2018 at 05:41 Cedric Bail <ced...@ddlm.me>
> wrote:
> > > > > > > >
> > > > > > > > > > The whole efl_del argument just exist because it is kinda
> > > poorly
> > > > > > > > > > named. IMO, del means: get this object to an "empty"
> state.
> > > > > > > > > > Just like close to files and hide and unparent to UI
> objects.
> > > > > efl_del
> > > > > > > > > > should not steal references under people who owns it, the
> > > object
> > > > > > > > > > would get deleted at a later time when everybody using
> the
> > > object
> > > > > > > > > > stops doing so, we could even return errors from
> efl_del'eted
> > > > > > > > > > objects for methods that do not make sense anymore,
> causing
> > > > > > > > > > most actions to, possibly, halt earlier rather than
> later.
> > > > > > > > >
> > > > > > > > > So, if efl_del does not still a references under people who
> > > owns
> > > > > it,
> > > > > > > how
> > > > > > > > > do we fix it ? Should it still magically reset its parent
> to
> > > NULL
> > > > > when
> > > > > > > > > there is one and just efl_unref in the other case ? Should
> it
> > > be
> > > > > > > symetric
> > > > > > > > > to efl_add_ref and always reset the parent to NULL along
> with
> > > > > unref ?
> > > > > > > Or
> > > > > > > > > should it do none of this at all and you have to manually
> do
> > > the
> > > > > > > parent set
> > > > > > > > > and the unref ?
> > > > > > > > >
> > > > > > > > > Trying to figure out what behavior would make it work for
> > > binding,
> > > > > I
> > > > > > > would
> > > > > > > > > guess it would be best to just make it symetric to
> efl_add_ref.
> > > > > This
> > > > > > > will
> > > > > > > > > give a predictable outcome I think, but I am not sure it is
> > > enough.
> > > > > > > What do
> > > > > > > > > you think ?
> > > > > > > > >
> > > > > > > > > > IMO, the whole problem with efl_add/efl_add_ref is that
> > > > > > > > > > "parents" are treated specially, which they should not.
> > > > > parent_set
> > > > > > > > > > should increment efl_ref and parent_unset should
> decrement
> > > it.
> > > > > > > > >
> > > > > > > > > Agreed and surprised it is not the case.
> > > > > > > > >
> > > > > > > > > > For C, OTH, where we do expect some "automatism" on
> resource
> > > > > > > > > > handling, efl_unref'ing may be too much of a hassle when
> a
> > > > > > > > > > parent is already going to handle the lifetime of the
> > > object. So,
> > > > > > > > > > it would make sense, IMO, for efl_add_scope. It could
> even be
> > > > > > > > > > that efl_add_scope is named efl_add, no problem, as long
> as
> > > > > > > > > > there's a efl_add that keeps this semantics for binding
> > > > > > > > > > development. Which also means that parent_set/unset must
> > > > > > > > > > be fixed.
> > > > > > > > >
> > > > > > > > > I think that once efl_del behavior is clearly defined, the
> > > > > existence
> > > > > > > of an
> > > > > > > > > efl_add_scope/efl_add will also be clearer to everyone.
> > > > > > > > >
> > > > > > > > > > Also, @own tags must not relate to parent_set, because
> that
> > > > > > > > > > has no useful information for tags or users, if needed
> we can
> > > > > > > > > > add a @reparent tag, but that's not really special
> > > information
> > > > > > > > > > such as real owernship information.
> > > > > > > > >
> > > > > > > > > I am still wondering what the @own really mean. Does that
> mean
> > > > > that the
> > > > > > > > > object own at least one reference of it ? But in that case,
> > > doesn't
> > > > > > > that
> > > > > > > > > mean that the user need to always ref it, if it plans to
> keep
> > > it
> > > > > > > around.
> > > > > > > > >
> > > > > > > > > As for @reparent, I am not sure we have case yet where we
> > > return an
> > > > > > > object
> > > > > > > > > that can not be reparented, do we have such a case ?
> > > > > > > > >
> > > > > > > > > Cedric
> > > > > > > > >
> > > > > > > > >
> > > > > > > ------------------------------------------------------------
> > > > > ------------------
> > > > > > > > > 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
> > > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > --
> > > > > > > > http://andywilliams.me
> > > > > > > > http://ajwillia.ms
> > > > > > > >
> > > > > > > ------------------------------------------------------------
> > > > > ------------------
> > > > > > > > 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-d
> evel
> > > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > --
> > > > > > > ------------- Codito, ergo sum - "I code, therefore I am"
> > > > > --------------
> > > > > > > Carsten Haitzler - ras...@rasterman.com
> > > > > > >
> > > > > > >
> > > > > >
> > > > > > --
> > > > > > http://andywilliams.me
> > > > > > http://ajwillia.ms
> > > > >
> > > > >
> > > > > --
> > > > > ------------- 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
> > > > >
> > > > >
> > > >
> > > > PS: The command lines are as follows (remove the grep -v for the
> first
> > > > stats):
> > > >
> > > > git grep --color=never -E "efl_add\(.*CLASS," |grep -v
> EFL_UI_WIN_CLASS
> > > > |grep -v ECORE |sed -e 's/.*efl_add(\([^,]\+\),.*/\1/' |sort -u|wc
> > > > git grep --color=never -E "efl_add\(.*CLASS," |grep -v
> EFL_UI_WIN_CLASS
> > > > |grep -v ECORE |sed -e 's/.*efl_add(\([^,]\+\),.*/\1/' |wc
> > > >
> > > > git grep --color=never -E "efl_add\(.*CLASS, NULL" |grep -v
> > > > EFL_UI_WIN_CLASS |grep -v ECORE |sed -e
> 's/.*efl_add(\([^,]\+\),.*/\1/'
> > > > |sort -u|wc
> > > > git grep --color=never -E "efl_add\(.*CLASS, NULL" |grep -v
> > > > EFL_UI_WIN_CLASS |grep -v ECORE |sed -e
> 's/.*efl_add(\([^,]\+\),.*/\1/'
> > > |wc
> > > >
> > > >
> > > > --
> > > > Jean-Philippe André
> > > > ------------------------------------------------------------
> > > ------------------
> > > > 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
> > >
> > >
> > > --
> > > ------------- Codito, ergo sum - "I code, therefore I am"
> --------------
> > > Carsten Haitzler - ras...@rasterman.com
> > >
> > >
> >
> >
> > --
> > Jean-Philippe André
> > ------------------------------------------------------------
> ------------------
> > 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
>
>
> --
> ------------- Codito, ergo sum - "I code, therefore I am" --------------
> Carsten Haitzler - ras...@rasterman.com
>
>
>
>
>
--
Jean-Philippe André
------------------------------------------------------------------------------
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
