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.
This makes eo print a WRN message in case a function is called on NULL.
efl_del is an exception to this rule (implemented in a hackish way, I
admit). I don't know any language or object model where using a null
object doesn't result in an exception or crash, except EO. In any case,
calls to null are invalid.
The next commits will resolve most warnings for EFL.
Ref T6326
Summary:
ecore_evas: remove debug
eina: unregister log level when done with
Fixes a constant memory leak.
eina: introduce EINA_HOT and EINA_COLD
These attributes respectivelly expand to __attribute__ ((hot)) and
__attribute__ ((cold)) when available. They allow to mark functions are
being hot/cold (frequently used or not) as well as to qualify labels
within a function (likely/unlikely branches).
eo: speed-up generated calls by removing call cache
The call cache needed to by thread-local, to avoid concurrency issues.
Problem with TLS is that is adds an extra overhead, which appears to be
greater than the optimization the cache provides.
Op is naturally atomic, because it is an unsigned integer. As such, it
cannot be tempered with while another thread is reading it. When
entering the generated function, the first operation done is reading
'op'. If we have concurrency, we will have access sequences returning
either EFL_NOOP or a VALID op, because 'op' is not set until the very
end of the function, when everything has been computed. As such, we now
use the 'op' atomic integer to instore a lock-free/wait-free mechanism,
which allows to drop the TLS nature of the cache, speeding up the access
to the cache, and therefore making functions execute faster.
We don't test anymore the generation count. This can be put as a
limitation. If means that if you call efl_object_shutdown() and
re-initialize it later with different data, opcodes will be invalid.
I am not sure there is any usecase for this to ever happen.
We could move all the caches in a dedicated section, that can be
overwritten after a call to efl_object_shutdown(), but I am not sure it
will be very portable.
Benchmark: mean over 3 executions of
ELM_TEST_AUTOBOUNCE=100 time elementary_test -to genlist
```
BEFORE AFTER
------------------------------------------------------------
time (ns) 11114111647.0 9147676220.0
frames 2872.3333333333335 2904.6666666666665
time per frame (ns) 3869364.6666666665 3149535.3333333335
user time (s) 11.096666666666666 9.22
cpu (%) 22.666666666666668 18.333333333333332
```
Ref T6580
Reviewers: raster, cedric
Subscribers: cedric, jpeg
Maniphest Tasks: T6580
Differential Revision: https://phab.enlightenment.org/D5738
we can't sensibly use things like massif to track memory if we bypass
itr with mmaping -1 fd anonymous memory... so if built with valgrind
support and running under valgrind, use malloc/calloc and free so
these tools actually do something useful for these bits of memory.
in some cases construction can fail badly and this causes a crash...
eoid is NULL going in and this causes a cascade of failure. survive
and be more robust.
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).
It would be pretty stupid to create an object that dies before it even
gets created. Auto-unref is for parts and should probably not be used
anywhere else :)
the freeq can't allow things to access the content of the pointer
because the content canbe written over by canary handling qhen
submitted to the freeq. clear it inline then just submit the final
pointer to the freeq.
@fix
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
export EO_LIFECYCLE_BACKTRACE=0
This means that the lifecycle can be tracked but we don't care about the
backtraces. Related to some recent eo_debug patches.
If an object still has a parent inside the del intercept, we shouldn't
reset the "parent_sunk" flag. This would indeed break logic as
parent_sunk == false implies that a parent was never set, which means
the parent must be null. Right now this case isn't used but it can be
imagined with caches of evas objects (they should remain parented to
their evas).
@fix
efl_data_scope_safe_get() is often used to assert that an object is of a
certain type, but assuming it may be NULL or not of the required type.
This means that encountering an invalid type is an error handled by the
caller (we return NULL), and shouldn't print out extra ERR() logs.
This fixes issues with E run under eo_debug. E's code was safe as it's
using evas_object_smart_data_get() and verifies whether the returned
value is NULL (which is expected for a rectangle, for instance).
@fix
This is done via the syntax (not documented yet):
EO_LIFECYCLE_DEBUG=N:<class names>
Where N is:
1. new/free
2. new/free/ref/unref
3. new/free/ref/unref/reuse
@feature
It's not enough to check the init generation count, if the cache data is
not reset properly.
This patch adds safety in two ways:
1. Actually reset the op call cache when we detect a new generation
2. Verify that we don't return NULL function pointer
The 1st point is absolutely necessary, 2nd is optional (extra cost of
one if()).
@fix
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.
There is a problem with the previous version. The object can still be
alive due to the use of manual_free in evas. So you wouldn't be able
for example to remove a callback from an object that hasn't been
destroyed yet. If that callback is triggered by the destruction
of the object, you would end up with an unexpected and impossible to
prevent effect of access after free on a callback that you had removed.
Not sure if that still solve the original problem that the code was
trying to prevent in Ecore_Evas.
This makes sure the object is actually still alive and kicking before
returning any data. Otherwise the "safe" word is a bit of an abuse...
Ref T5869
@fix
In some case, detected during eo test suite, the vtable does fail to
be fully assigned, but it is still being assigned as the new vtable.
Of course when later destroying it, it has already been freed. Leading
to a double free.
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
This allows two things:
- adding new override functions on an object that already has
overrides
- resetting a specific function (or list of functions) to the
parent class implementation by passing NULL as implementation
Fixes T5580
@feature
Explicit lock / unlock of the shared mutex mixed with implicit
lock / unlock when accessing the internal Eo_Object data lead
to uncaught issues such as these.
This was found by trying to run E with gfx filters under eo_debug.
eo_function(NULL) always leads to a no-operation. A this
point it is basically considered the normal operation
with EO to just silently ignore NULL errors.
If the API function "eo_function" belongs to a class that
has not been loaded yet (eg. no object of this type has
been created yet), then the op associated to the API func
may not be defined yet.
Fixes T5715
This is a safe version of efl_data_scope_get, meaning that it will
return NULL if the object is not of the required type, or if there
is no data for that class, or if the given class was used as an
interface (and isn't a mixin).
@feature
This makes it work like C++ dynamic_cast<> operator, so that
the return value will be NULL if the object is not an instance
of the given class.
In case of efl_super() we don't do it as efl_super is used A LOT
inside EFL itself (all constructors & destructors, for a start)
and efl_isa is in fact a bit expensive. efl_cast isn't really used
and is intended to be something like dynamic_cast.
For @cedric :)
This is similar to efl_super but the specified class is the one
we want to call the function on. This is similar to dynamic_cast<>
in C++.
Note: both efl_super() and efl_cast() need documentation!
This is an experimental feature.
Fixes T5311
@feature
Maniphest Tasks: T5311
Differential Revision: https://phab.enlightenment.org/D4797
I've always really disliked this lock. If someone calls a non-eo
function by accident with efl_super() then you'll most likely end
up in a deadlock.
This adds the cur_klass pointer to the object itself, exploiting
the fact that we have 8 bytes of padding (on 64 bits, at least).
Also, this opens the door to efl_cast() which would be similar to
efl_super() except that only a dynamic cast is done, not a call
to the parent function.
make benchmark shows a performance improvement, surprisingly.
This is a bit experimental. See also the following commit (efl_cast)
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
If efl_object_override() is called with a function that does
not exist in the original class, it may lead to a crash on
indexing an non-existing array in the vtable.
This is really just a safety check, as the usage was wrong:
* You are only allowed to override functions that are defined in the
* class or any of its interfaces (that is, efl_isa returning true).
In case of manual free, as is heavily used by Evas, we can't really
print an ERR if there are still references before free has been
called.
This may not be ideal from a pure EO point of view but considering
how Evas uses manual free this is the best solution to avoid
polluting debug logs.
This changes the following message when the object is referencing
its own data. Also lowers from ERR to WRN and adds the class
name for the referenced data.
ERR<17450>:eo /home/jpeg/e/core/efl/src/lib/eo/eo_private.h:337
_efl_unref_internal() in /home/jpeg/e/core/efl/src/lib/eo/eo.c:620:
func '_efl_object_call_end' Data of object 0x400000021008db58 is
still referenced by object 0x400000021008db58
Note that evas and elm have a few calls to efl_data_ref(obj, NULL)
which are imho quite ugly: not using the return value and not
specifying the data class. I'm keeping them as-is for now.
This condition can never be true. It can't be NULL here. A NULL here
would have caused a crash earlier, though it can only be NULL if an
allocation fails, which is something that we don't really handle
for smallish allocations.
CID1366823
Since we keep a log of created and deleted objects, we can walk the
log and see which were leaked. As this is expensive, do only if log
level is greater than 3 (INFO, DEBUG...), with backtrace of object
creation being displayed as backtrace if running as level 4 (DEBUG).
This fixes an issue where efl_isa() wouldn't work for extensions or
ancestors of extensions of a class.
Example:
Class A implements interface F2
F2 inherits from interface F1
obj is of class A
Before this patch efl_isa(obj, F1) would return false, now it returns
true as expected.
This is just one example, there is a whole array of variations to this
issue that are now fixed.
Thanks to Gustavo for reminding me of this.
@fix
Tom is worried about performance hit (god, checking a bit in a pointer
we'll fetch to memory anyway, since we return it masked), so guard
under EO_DEBUG.
When writing this ERR log I thought about "thread" (it's really
the keyword here) but eventually reworded to "context". Let's be
clearer about the possible issue here.
_efl_object_api_op_id_get() will query a hash for the given pointer,
however if it wasn't populated, it will return "NOOP" and we're
hopeless while debugging on what happened.
Common case is to use the incorrect method, like:
obj = efl_add(CLS1, ...);
cls2_method(obj);
Since we did not create CLS2, it won't populate its methods on the
hash, thus the lookup will return NOOP.
With this change the function now gets the target object and function
name so reports an insightful message such as:
ERR:eo file.c:123 cls2_method() Unable to resolve op for api func 0x7ff492ddea00 for obj=0x400000007e8ee1df (CLS1)
Eo pointer indirection is super nice as it avoids you to access
invalid memory, but this extra checks inhibits valgrind's own tracking
of memory lifecycle, usually it would report when the object was
created and when the object is deleted, both as stack traces.
This commits introduces logging of object creation and destruction
under its own eina_log_domain and controlled by EO_LIFECYCLE_DEBUG and
EO_LIFECYCLE_NO_DEBUG envvars. These will only be available if
compiled with EO_DEBUG, thus shouldn't cause any performance hits on
production code.
Running a bogus app with invalid efl_class_name_get() and double
efl_del() will report as below:
```sh
$ export EO_LIFECYCLE_NO_DEBUG=Efl_Loop_Timer,Efl_Promise,Efl_Future
$ export EO_LIFECYCLE_DEBUG=1
$ export EINA_LOG_LEVELS=eo_lifecycle:4
$ /tmp/bogus_app
DBG:eo_lifecycle lib/eo/eo.c:2712 _eo_log_obj_init() will log all object allocation and free
DBG:eo_lifecycle lib/eo/eo.c:2788 _eo_log_obj_init() will NOT log class 'Efl_Future'
DBG:eo_lifecycle lib/eo/eo.c:2788 _eo_log_obj_init() will NOT log class 'Efl_Promise'
DBG:eo_lifecycle lib/eo/eo.c:2788 _eo_log_obj_init() will NOT log class 'Efl_Loop_Timer'
DBG:eo_lifecycle lib/eo/eo.c:2665 _eo_log_obj_new() new obj=0x563fa35a1aa0 obj_id=0x4000000002cf38ef class=0x563fa35a1450 (Efl_Vpath_Core) [0.0004]
DBG:eo_lifecycle lib/eo/eo.c:2665 _eo_log_obj_new() new obj=0x563fa35af8d0 obj_id=0x4000000006cf38f0 class=0x563fa35aecf0 (Efl_Loop) [0.0005]
DBG:eo_lifecycle lib/eo/eo.c:2665 _eo_log_obj_new() new obj=0x563fa35d61a0 obj_id=0x400000007ecf390e class=0x563fa35d48f0 (Efl_Net_Dialer_Simple) [0.0054]
DBG:eo_lifecycle lib/eo/eo.c:2665 _eo_log_obj_new() new obj=0x563fa35d6470 obj_id=0x4000000082cf390f class=0x563fa35d0d60 (Efl_Net_Dialer_Tcp) [0.0055]
DBG:eo_lifecycle lib/eo/eo.c:2665 _eo_log_obj_new() new obj=0x563fa35d75b0 obj_id=0x4000000086cf3910 class=0x563fa35d66b0 (Efl_Io_Queue) [0.0056]
DBG:eo_lifecycle lib/eo/eo.c:2665 _eo_log_obj_new() new obj=0x563fa35d8f70 obj_id=0x400000008acf3911 class=0x563fa35d7860 (Efl_Io_Copier) [0.0057]
DBG:eo_lifecycle lib/eo/eo.c:2665 _eo_log_obj_new() new obj=0x563fa35df980 obj_id=0x40000000a6cf3918 class=0x563fa35d66b0 (Efl_Io_Queue) [0.0058]
DBG:eo_lifecycle lib/eo/eo.c:2665 _eo_log_obj_new() new obj=0x563fa35dfc30 obj_id=0x40000000aacf3919 class=0x563fa35d7860 (Efl_Io_Copier) [0.0058]
will efl_class_name_get() with invalid handle:
ERR:eo lib/eo/eo.c:1013 efl_class_name_get() Class (0x2000000000000029) is an invalid ref.
ERR:eo_lifecycle lib/eo/eo.c:1013 efl_class_name_get() obj_id=0x2000000000000029 was neither created or deleted (EO_LIFECYCLE_NO_DEBUG='Efl_Loop_Timer,Efl_Promise,Efl_Future').
DBG:eo_lifecycle lib/eo/eo.c:2688 _eo_log_obj_free() free obj=0x563fa35df980 obj_id=0x40000000a6cf3918 class=0x563fa35d66b0 (Efl_Io_Queue) [0.0061]
DBG:eo_lifecycle lib/eo/eo.c:2688 _eo_log_obj_free() free obj=0x563fa35dfc30 obj_id=0x40000000aacf3919 class=0x563fa35d7860 (Efl_Io_Copier) [0.0061]
DBG:eo_lifecycle lib/eo/eo.c:2688 _eo_log_obj_free() free obj=0x563fa35d75b0 obj_id=0x4000000086cf3910 class=0x563fa35d66b0 (Efl_Io_Queue) [0.0061]
DBG:eo_lifecycle lib/eo/eo.c:2688 _eo_log_obj_free() free obj=0x563fa35d8f70 obj_id=0x400000008acf3911 class=0x563fa35d7860 (Efl_Io_Copier) [0.0061]
DBG:eo_lifecycle lib/eo/eo.c:2688 _eo_log_obj_free() free obj=0x563fa35d6470 obj_id=0x4000000082cf390f class=0x563fa35d0d60 (Efl_Net_Dialer_Tcp) [0.0063]
DBG:eo_lifecycle lib/eo/eo.c:2688 _eo_log_obj_free() free obj=0x563fa35d61a0 obj_id=0x400000007ecf390e class=0x563fa35d48f0 (Efl_Net_Dialer_Simple) [0.0063]
will double free:
ERR:eo ../src/lib/eo/efl_object.eo.c:78 efl_del() EOID 0x400000007ecf390e is not a valid object. EOID domain=0, current_domain=0, local_domain=0. EOID generation=2cf390e, id=1f, ref=1, super=0. Thread self=main. Available domains [0 1 ]. Maybe it has been deleted or does not belong to your thread?
ERR:eo_lifecycle ../src/lib/eo/efl_object.eo.c:78 efl_del() obj_id=0x400000007ecf390e created obj=0x563fa35d61a0, class=0x563fa35d48f0 (Efl_Net_Dialer_Simple) [0.0054s, 0.0009 ago]:
ERR:eo_lifecycle ../src/lib/eo/efl_object.eo.c:78 efl_del() 0x007f2c0bc6d0ea: libeo_dbg.so+0x90ea (in src/lib/eo/.libs/libeo_dbg.so 0x7f2c0bc64000)
ERR:eo_lifecycle ../src/lib/eo/efl_object.eo.c:78 efl_del() 0x007f2c0bc6ca62: _efl_add_internal_start+0x1c2 (in src/lib/eo/.libs/libeo_dbg.so 0x7f2c0bc64000)
ERR:eo_lifecycle ../src/lib/eo/efl_object.eo.c:78 efl_del() 0x00563fa15dc95f: bogus_app+0x295f (in /tmp/bogus_app 0x563fa15da000)
ERR:eo_lifecycle ../src/lib/eo/efl_object.eo.c:78 efl_del() 0x007f2c0ace7291: __libc_start_main+0xf1 (in /usr/lib/libc.so.6 0x7f2c0acc7000)
ERR:eo_lifecycle ../src/lib/eo/efl_object.eo.c:78 efl_del() 0x00563fa15dc48a: _start+0x2a (in /tmp/bogus_app 0x563fa15da000)
ERR:eo_lifecycle ../src/lib/eo/efl_object.eo.c:78 efl_del() obj_id=0x400000007ecf390e deleted obj=0x563fa35d61a0, class=0x563fa35d48f0 (Efl_Net_Dialer_Simple) [0.0063s, 0.0000 ago]:
ERR:eo_lifecycle ../src/lib/eo/efl_object.eo.c:78 efl_del() 0x007f2c0bc6d8ba: libeo_dbg.so+0x98ba (in src/lib/eo/.libs/libeo_dbg.so 0x7f2c0bc64000)
ERR:eo_lifecycle ../src/lib/eo/efl_object.eo.c:78 efl_del() 0x007f2c0bc6d711: libeo_dbg.so+0x9711 (in src/lib/eo/.libs/libeo_dbg.so 0x7f2c0bc64000)
ERR:eo_lifecycle ../src/lib/eo/efl_object.eo.c:78 efl_del() 0x007f2c0bc6beb8: libeo_dbg.so+0x7eb8 (in src/lib/eo/.libs/libeo_dbg.so 0x7f2c0bc64000)
ERR:eo_lifecycle ../src/lib/eo/efl_object.eo.c:78 efl_del() 0x007f2c0bc6c06e: _efl_object_call_end+0x4e (in src/lib/eo/.libs/libeo_dbg.so 0x7f2c0bc64000)
ERR:eo_lifecycle ../src/lib/eo/efl_object.eo.c:78 efl_del() 0x007f2c0bc75725: efl_del+0x105 (in src/lib/eo/.libs/libeo_dbg.so 0x7f2c0bc64000)
ERR:eo_lifecycle ../src/lib/eo/efl_object.eo.c:78 efl_del() 0x00563fa15dcd54: lt-efl_net_dialer_simple_example+0x2d54 (in /tmp/bogus_app 0x563fa15da000)
ERR:eo_lifecycle ../src/lib/eo/efl_object.eo.c:78 efl_del() 0x007f2c0ace7291: __libc_start_main+0xf1 (in /usr/lib/libc.so.6 0x7f2c0acc7000)
ERR:eo_lifecycle ../src/lib/eo/efl_object.eo.c:78 efl_del() 0x00563fa15dc48a: _start+0x2a (in /tmp/bogus_app 0x563fa15da000)
ERR:eo_lifecycle ../src/lib/eo/efl_object.eo.c:78 efl_del() obj_id=0x400000007ecf390e was already deleted 0.0000 seconds ago!
```
if for some reason we fail to validate a class, then we should skip
that extension. This may result in an empty vtable, then check for
that and avoid a crash.
This is very unlike to happen in practice, but I've forced some
validation errors and could get to that.
Instead of 2 sets of macro, one for HAVE_EO_ID and another without,
use a single set of macros and have the implementation of
_eo_class_pointer_get() and _eo_obj_pointer_get() to do the actual
These functions now take the source information so the logs reflect
that and not always the same function.
_eo_pointer_error() was kinda a bitch to debug as it provided a nice
breakpoint location, but did not provide a good output since the file,
line and function were always the same.
Change that to be a thin wrapper on top of eina_log_vprint(), then we
keep the breakpoint location yet provide useful information.
In that sense, change other error messages so they carry as much
information as possible.
Normally when debugging Eo with gdb you can just use any of the internal
eo functions to resolve the id to its internal pointer. However, when
loading a coredump you can't execute any code, not even the id resolve
code.
This change adds a gdb function that resolves the id to its pointer form
without executing any code in the process space. This plugin is
essentially the id resolve code written in python as a gdb function.
Usage:
Print the pointer:
(gdb) print $eo_resolve(obj)
$1 = (_Eo_Object *) 0x5555559bbe70
Use it directly (e.g. to print the class name):
(gdb) $eo_resolve(obj)->klass->desc.name
This plugin requires that the coredump would be loaded with the exact
same libeo.so binary (or at least one that hasn't changed eo internals),
and that the debug symbols for libeo.so would be available for gdb to
use.
Note:
This feature is incomplete and only resolves IDs that are owned by the
main thread and in the main domain. This is not a big issue at the
moment, because almost all of our IDs are like that.
@feature
When resolving a function for an object the object would get reference,
and then in some failure cases won't be freed.
I suspect this is a regression following the reshuffling that was done
in that function recently.
Thanks to zmike for investigating and reporting this.
Fixes T4740
@fix
This happens with shared objects.
The situation seems to be:
1. object has composited object a of class A in thread 1
2. call something on object a from thread 2, deadlock
In fact, do anything from thread 2 on a shared object and you deadlock.
this moves a lot of error case handling into goto's so the code gets
out of the hot path and this should help expecially since variou
smacros do things like:
do { char buf[256]; sprintf(buf, fmt, ptr); _eo_pointer_error(buf); } while (0)
_Efl_Class *klass; \
do { \
klass = _eo_class_pointer_get(klass_id); \
if (!klass) { \
_EO_POINTER_ERR("Class (%p) is an invalid ref.", klass_id); \
return ret; \
} \
} while (0)
so putting quite a chunk of code inside a rare "if this errors"
handler that will cause l1 cache misses and this we don't want, thus
moving stuff in eo core out of hot paths to cut down on overhead. yes
it might not be pretty but it's kind of the right thing at such a core
level of efl. this also does the same to the eo base class as this is
also going to be relatively hot given it's the core of every other
object.
so there were a few issues. one we had a spinlokc on the eoid table
for shared objects AND then had a mutex for accessing those objects
(released on return from any eo function). BUT this missed some funcs
like eo_ref, eo_unref and so on in eo.c ... oops. so fixed. but then i
realized there was a race condition. we locked the eoid table then
unlocked with our pointer THEN locked the sharted object mutex ...
then unlocked it. that was a race condtion gap. so we should share the
same lock anyway - if it's a shared object, grab the shared object
mutex then do a lookup and if the lookup does not fail, KEEP the lock
until it is released by the return from eo function or by some special
macro/funcs that released a matching lock. since its a recursive lock
this is all fine. as its also a universal single lock for all objects
we just need the eoid to know if it's shared and needs locking based
on the domain bits. so now do this locking properly with just a single
mutex, not both a spinlock and mutex and keep the lock around until
totally done with the object. this plugs the race condition holes and
goes from 1 spinlock lock and unlock then a mutex lock and unlokc to
just a single mutex lock and unlock. this means shared objects are
actually truly safe across threads and only have the overhead of a
single recursive mutex to lock and unlock in every api call.
Summary:
as told in _eina_stringshared_key_cmp in eina_hash.c:
originally we want to do this:
return key1 - key2;
but since they are ptrs and an int can't store the different of 2 ptrs in
either 32 or 64bit (signed hasn't got enough range for the diff of 2
32bit values regardless of their type... we'd need 33bits or 65bits)
So changing this to the same logic.
Reviewers: tasn, raster
Subscribers: cedric, jpeg
Differential Revision: https://phab.enlightenment.org/D4298
This change lets us remove a field from the structure that leads to
around 20KiB more of saving in private dirty pages in elementary.
This also looks a bit better and feels a bit cleaner.
Breaks API and ABI.
Before this commit, function overrides were explicit. That is, you'd
have to explicitly state you were overriding a function instead of
creating a new one. This made the code a tad more complex, and was also
a bit more annoying to use. This commit removes this extra piece of
information.
This means we now store much less information per function, that will
let us further optimise out structures in the future.
Now that we have recursive locks, the class creation code can be much simpler.
All the code there was essentially our own implementation of recursive locks,
or rather a special case of those.
This is no longer needed.
Cedric BAIL
Cedric BAIL
Jean Guyomarc'h
Jean-Philippe Andre
Carsten Haitzler
Andy Williams
Vincent Torri
Guilherme Iscaro
Daniel Zaoui
Gustavo Sverzut Barbieri
Tom Hacohen
Marcel Hollerbach