Summary:
This will avoid infinite loops and errors when the parent tries
to orphan an invalidated child.
Fixes T6780
Test Plan: Run `make check`
Reviewers: cedric
Maniphest Tasks: T6780
Differential Revision: https://phab.enlightenment.org/D5839
Reviewed-by: Cedric BAIL <cedric@osg.samsung.com>
This is just a first step. All user of destructor should be updated to
move the code that rely on their efl_parent and on efl_provider_find to
invalidate. Then we will be able to change the way efl_add and efl_del
work to properly refcount things. efl_noref won't be triggered at the
moment until both efl_parent_set(obj, NULL) and the last user ref are
set to NULL. This is not what we want, but due to how user refcount is
accounting parent at the moment, until all the code is move to rely
on invalidate we can not fix this.
so the MAIN loop is actually an efl.app object. which inherits from
efl.loop. the idea is that other loops in threads will not be efl.app
objects. thread on the creator side return an efl.thread object.
inside the thread, like the mainloop, there is now an efl.appthread
object that is for all non-main-loop threads.
every thread (main loop or child) when it spawns a thread is the
parent. there are i/o pipes from parnet to child and back. so parents
are generally expected to, if they want to talk to child thread, so
use the efl.io interfaces on efl.thread, and the main loop's elf.app
class allows you to talk to stdio back to the parent process like the
efl.appthread does the same using the efl.io interfaces to talk to its
parent app or appthread. it's symmetrical
no tests here - sure. i have been holding off on tests until things
settle. that's why i haven't done them yet. those will come back in a
subsequent commit
for really quick examples on using this see:
https://phab.enlightenment.org/F2983118https://phab.enlightenment.org/F2983142
they are just my test code for this.
Please see this design document:
https://phab.enlightenment.org/w/efl-loops-threads/
Triggered after (almost) complete destruction of the object.
Not called "deleted" because the other event is already "del".
I don't like "destruct" much but this follows the terminology of
"constructor" / "destructor".
@feature
This API is meant to be used by parts only, and by bindings dealing with
part objects. This patch fixes make check which got broken in the after
the previous one (cxx).
Before screaming in horror (C++...) here's why we may need this:
Efl.Part.part API returns an object that is by definition valid for a
single function call only. Enforcing this in practice is actually quite
hard as all implementation functions must manually take care of the
life-cycle. This is a lot of code in many places and a lot of
opportunities to forget to properly handle that life-cycle. Also, this
means any invalid function call on a part will leak an object.
This API absolutely must remain either "internal" or "beta" and
definitely not become abused by applications. On top of that such an API
can cause great trouble for bindings like C++. As a consequence, only
specially crafted APIs like efl_part() should return an object marked as
auto_unref.
Alternatively this API could be defined in Eo.h or some other
Eo_Internal.h. I placed it in efl_object.eo because it's much more
convenient :) (read: I'm lazy)
****
Performance notes:
Tested with clang & gcc (with -O2), I had a look at the output of perf
top, in particular the asm view. I used eo_bench in a loop. My
conclusions are:
- EINA_LIKELY/UNLIKELY actually works. The jump statement varies
according to the expectation. I highly doubt all those ugly goto in
eo.c / Eo.h are even useful.
- The impact of auto_unref on a call_resolve is so small it doesn't even
appear in the trace. It is significant inside call_end, though
(obviously, that function is just a few lines long). That function
accounts for ~1% to ~4% of all CPU time. The impact of auto_unref in
call_end is ~4% of the function time. This means ~0.16% of all CPU
time (worst measured case). _efl_object_op_api_id_get simply doesn't
show up because of caching, so the extra check there is negligible.
PS: I also tested EINA_LIKELY/UNLIKELY by compiling with -O2 and looking
at the output with objdump. The flag is well respected, and the jump
instructions are what you would expect (no jump for LIKELY and jump for
UNLIKELY). Conclusion: The goto's in eo.c only make the code harder to
read...
Simply pass in the strbuf and don't expect the callee to own it. This
makes things simpler and safer (it'll crash only if the callee frees
said strbuf, and shouldn't leak). efl_ebug_name is new in the upcoming
release, EFL 1.21.
Realised this after talking with Amitesh. Thanks.
See 999dbd9764
And c4769ff898
This allows deleting an object by simply calling efl_unref() on it, even
if there is a parent. Normally the parent is in charge, and you can
request deletion by calling efl_del() or efl_parent_set(NULL).
But in some rare cases, you want to give ownership of an object (@owned)
and still give a parent to that object. efl_unref() should be used (and
would be used by bindings when going out of scope or on garbage
collection), which would then print an error message. This API allows
the specific behaviour.
@feature
This commit adds the EO support for the new future infra.
From now on there's no need to efl_future_link()/efl_future_unlink()
object and futures since the new API already handles that internally.
This will include the following information, by default:
- class name
- whether the class is an override
- eo id (pointer)
- refcount
- name if one was set (Efl.Object property)
This also supports classes, which is why it's an EAPI in eo.c
and not only a method of Efl.Object
This can be overriden by subclasses using the empty method
Efl.Object.debug_name_override.get
If the function is overriden, then the returned string is used
as is and so it is left to the subclass to include all the
necessary information (as above). This can easily be achieved
by calling efl_debug_name_get(efl_super()) and then concatenating
the strings.
Think of this function as something like Java's toString(), but
only for debugging (i.e. a string class should not just return
its string value).
@feature
There have been cases where the logic of _event_callback_call break'ed
too early in the event submission.
Reason for that was the line ((const unsigned char *) desc -
(const unsigned char *) it->desc) producing a overflow.
This means the if statement
if (!legacy_compare &&
((const unsigned char *) desc - (const unsigned char *) it->desc) < 0)
was true while the pointer desc was smaller than it->desc, which means
the event subscription got aborted, even if it should not.
This turned out on two 32 bit maschines. And led to not rendering apps
anymore.
It was introduced by commit in 605fec91ee.
@fix
In a few classes, this requires some manual expansion. This should
not break anything but it's also fairly ugly; a better solution
would be appreciated, for now we do this.
Similar changes will be done to a few other Efl.Object APIs as
well at later point.
Summary:
This uses Eina_Cow to implement support for rarely used features
in EO objects. This covers:
- composite objects (eg. UI widgets: combobox, text, video, win)
- vtable for efl_object_override
- del_intercept
All of these features are quite tricky to get right and while
very useful, should still be used with great care. With this patch,
the size of an _Eo_Object struct comes down from 80 bytes (rounded
up from 72b) to 64 bytes (rounded up from 56b) on 64 bits.
Also I haven't measured precisely but I don't expect any performance
impact since the COW data is more likely to remain in L1/L2 cache,
as the default one will be used most often. Unfortunately, the
results of "make benchmark" have been quite inconsistent over
multiple runs.
This saves ~64kb in elementary_test (>4k objects) at the cost of
~100 calls to COW write (del intercept on some events).
@optimization
Reviewers: raster, cedric
Differential Revision: https://phab.enlightenment.org/D4796
Summary:
when a few recursive event emissions are happening, and in some deep
recursive level a subscription to the same object is happening, the
subscription would just be executed when the complete recursion is done.
that is wrong. The subscription needs to be executed when the event is
called after the subscription is added, undepended from any recursive
level. That fixes that and adds a regression test for it.
This was discovered in e, since e gives a lot of error messages about a eo object
that is already freed. It turned out this object is returned from evas, and exactly
the above happened to the EFL_EVENT_DEL subscription of that object.
Test Plan: make check
Reviewers: tasn, cedric, stefan_schmidt
Subscribers: stefan_schmidt, netstar, zmike, raster, jpeg
Differential Revision: https://phab.enlightenment.org/D4656
Signed-off-by: Cedric BAIL <cedric@osg.samsung.com>
yes - we compare a difference between 2 ptrs and an index which is a
uint. the safe thing here is to promote the unit to the ptrdiff_t
type. reality is we cant have more than 2^32 cb's on an object
anyway... so this should be ok.
This fixes T4907
The problem was that in efl_event_callback_add the internal array was
changed. If this was happening while a efl_event_callback_call was
happening the for loop got confused and skipped one event subscription.
Which led to a bug in e where the idler ufnction was not executed
probebly and so the canvas stayed frozen.
if there is a event the callback counter should be incremented not
decremented. This should fix a few crashes i found in edje, since edje
did not knew that a element was deletion.
so hunting another callback issue i noticed some of THE most popular
callbacks are:
1411 tick
1961 move
4157 pointer,move
7524 dirty
8090 damage
13052 render,flush,post
13052 render,flush,pre
13205 render,post
13205 render,pre
21706 recalc
21875 idle
27224 resize
27779 del
31011 idle,enter
31011 idle,exit
60461 callback,del
104546 callback,add
126400 animator,tick
as you can see callback del, add and the general obj del cb's are
right up there... so it is very likely a good idea to CHECK to see if
anyone is listening before calling the callback for these very very
very common calls.
this is ifdef'd and turned on for now. it can be turned off. it
shouldnt use more memory due to the way memory alignment works (likely
all allocations will be multiples of 8 bytes anyway) so we're using
spare unused space. the only q is - is management of the counts AND
checking them worth it in general? it's really hard to tell given we
dont have a lot of benchmarks that cover a lot of use cases... it
doesnt seem to slow or speed anything up much in the genlist bounce
test... so i think i need something more specific.
@optimize
i found a massive slowdown that over time ended up with 10000's of
cb's in objects like the ecore loop object. this fixes that by
ACTUALLY flagging event deletions waiting to be true rather than false.
Cedric Bail
Lauro Moura
Cedric BAIL
Carsten Haitzler
Jean Guyomarc'h
Jean-Philippe Andre
Guilherme Iscaro
Marcel Hollerbach
Daniel Kolesa
Andy Williams