eo: Add beta API for auto_unref after a call

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...
This commit is contained in:
Jean-Philippe Andre 2017-10-13 17:18:41 +09:00
parent a1a04629e4
commit df304d0155
5 changed files with 115 additions and 3 deletions

View File

@ -279,6 +279,29 @@ abstract Efl.Object ()
even if @.parent is not $null.]]
}
}
@property auto_unref @protected @beta {
[[Mark an object to be automatically deleted after a function call.
This becomes effectives only after finalize is done. After any EO
function has been called on this object, it will be unref'ed. This
property will also be reset to $false. This is intended to simplify
Part objects lifecycle.
Note: This applies to any EO function call, even if the object was
of the wrong type (call resolution failed).
This is a write-only property as reading it would unref the object,
and reset the flag.
Warning: This is a beta API, do not use it unless you know exactly
what you are doing.
]]
set {}
values {
enable: bool(false);
[[If $true, unref this object after the next call.]]
}
}
}
implements {
class.constructor;

View File

@ -16,6 +16,9 @@
# include <mach/mach_time.h>
#endif
#define EFL_OBJECT_BETA
#define EFL_OBJECT_PROTECTED
#include "Eo.h"
#include "eo_ptr_indirection.h"
#include "eo_private.h"
@ -616,6 +619,11 @@ err_func_src:
err:
if (is_obj)
{
if (EINA_UNLIKELY(obj->auto_unref != 0))
{
if (!(--obj->auto_unref))
efl_unref(eo_id);
}
_efl_unref(obj);
_eo_obj_pointer_done((Eo_Id)eo_id);
}
@ -670,6 +678,11 @@ _efl_object_call_end(Efl_Object_Op_Call_Data *call)
{
if (EINA_LIKELY(!!call->obj))
{
if (EINA_UNLIKELY(call->obj->auto_unref != 0))
{
if (!(--call->obj->auto_unref))
efl_unref(call->eo_id);
}
_efl_unref(call->obj);
_eo_obj_pointer_done((Eo_Id)call->eo_id);
}
@ -721,19 +734,26 @@ _efl_object_api_op_id_get(const void *api_func)
}
EAPI Efl_Object_Op
_efl_object_op_api_id_get(const void *api_func, const Eo *obj, const char *api_func_name, const char *file, int line)
_efl_object_op_api_id_get(const void *api_func, const Eo *eo_obj, const char *api_func_name, const char *file, int line)
{
Efl_Object_Op op;
#ifndef EO_DEBUG
if (!obj) return EFL_NOOP;
if (!eo_obj) return EFL_NOOP;
#endif
op = _efl_object_api_op_id_get_internal(api_func);
if (op == EFL_NOOP)
{
EO_OBJ_POINTER(eo_obj, obj);
eina_log_print(_eo_log_dom, EINA_LOG_LEVEL_ERR,
file, api_func_name, line,
"Unable to resolve op for api func %p for obj=%p (%s)", api_func, obj, efl_class_name_get(obj));
"Unable to resolve op for api func %p for obj=%p (%s)", api_func, eo_obj, efl_class_name_get(eo_obj));
if (EINA_UNLIKELY(obj->auto_unref))
{
if (!(--obj->auto_unref))
efl_unref(eo_obj);
}
return EFL_NOOP;
}
return op;

View File

@ -5,6 +5,9 @@
#include <Eina.h>
#include <fnmatch.h>
#define EFL_OBJECT_BETA
#define EFL_OBJECT_PROTECTED
#include "Eo.h"
#include "eo_ptr_indirection.h"
#include "eo_private.h"
@ -2106,6 +2109,14 @@ _efl_object_allow_parent_unref_get(Eo *obj_id EINA_UNUSED, Efl_Object_Data *pd)
return pd->allow_parent_unref;
}
EOLIAN static void
_efl_object_auto_unref_set(Eo *obj_id EINA_UNUSED, Efl_Object_Data *pd EINA_UNUSED, Eina_Bool enable)
{
// Write-only property
EO_OBJ_POINTER(obj_id, obj);
obj->auto_unref = enable ? 2 : 0;
}
EOLIAN static Eo *
_efl_object_finalize(Eo *obj, Efl_Object_Data *pd EINA_UNUSED)
{

View File

@ -119,6 +119,7 @@ struct _Eo_Object
Eina_Bool del_triggered:1;
Eina_Bool destructed:1;
Eina_Bool manual_free:1;
unsigned char auto_unref : 2; // unref after 1 call
};
/* How we search and store the implementations in classes. */

View File

@ -9,6 +9,9 @@
# include <unistd.h>
#endif
#define EFL_OBJECT_BETA
#define EFL_OBJECT_PROTECTED
#include <Eo.h>
#include "eo_suite.h"
@ -1719,6 +1722,59 @@ START_TEST(efl_cast_test)
}
END_TEST
static void
_auto_unref_del_cb(void *data, const Efl_Event *ev EINA_UNUSED)
{
*((int *) data) = 1;
}
START_TEST(efl_object_auto_unref_test)
{
int _auto_unref_del;
Eo *obj, *parent;
efl_object_init();
// Test unref after valid call
_auto_unref_del = 0;
obj = efl_add(SIMPLE_CLASS, NULL);
fail_if(efl_ref_get(obj) != 1);
efl_event_callback_add(obj, EFL_EVENT_DEL, _auto_unref_del_cb, &_auto_unref_del);
efl_auto_unref_set(obj, 1);
fail_if(_auto_unref_del);
fail_if(efl_ref_get(obj) != 1);
efl_name_set(obj, "name");
fail_if(!_auto_unref_del);
// Test unref after invalid call
_auto_unref_del = 0;
obj = efl_add(SIMPLE_CLASS, NULL);
fail_if(efl_ref_get(obj) != 1);
efl_event_callback_add(obj, EFL_EVENT_DEL, _auto_unref_del_cb, &_auto_unref_del);
efl_auto_unref_set(obj, 1);
fail_if(_auto_unref_del);
fail_if(efl_ref_get(obj) != 1);
simple_no_implementation(obj);
fail_if(!_auto_unref_del);
// Same with a parent
_auto_unref_del = 0;
parent = efl_add(SIMPLE_CLASS, NULL);
obj = efl_add(SIMPLE_CLASS, parent);
fail_if(efl_ref_get(obj) != 1);
efl_allow_parent_unref_set(obj, 1);
efl_event_callback_add(obj, EFL_EVENT_DEL, _auto_unref_del_cb, &_auto_unref_del);
efl_auto_unref_set(obj, 1);
fail_if(_auto_unref_del);
fail_if(efl_ref_get(obj) != 1);
efl_name_set(obj, "name");
fail_if(!_auto_unref_del);
efl_del(parent);
efl_object_shutdown();
}
END_TEST
void eo_test_general(TCase *tc)
{
tcase_add_test(tc, eo_simple);
@ -1744,4 +1800,5 @@ void eo_test_general(TCase *tc)
tcase_add_test(tc, eo_rec_interface);
tcase_add_test(tc, eo_domain);
tcase_add_test(tc, efl_cast_test);
tcase_add_test(tc, efl_object_auto_unref_test);
}