[E-devel] Eo object data referencing
Hi all, I pushed today the data referencing feature for Eo objects in efl and elementary. Check the commit log for explanations on why/how we do that. From now, eo_data_get is deprecated, so please use eo_data_scope_get, eo_data_ref or eo_data_xref instead. Thank you JackDanielZ (alias Daniel) -- Introducing AppDynamics Lite, a free troubleshooting tool for Java/.NET Get 100% visibility into your production application - at no cost. Code-level diagnostics for performance bottlenecks with 2% overhead Download for free and get started troubleshooting in minutes. http://p.sf.net/sfu/appdyn_d2d_ap1 ___ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
Re: [E-devel] Eo object data referencing
On 01/05/13 09:13, daniel.za...@samsung.com wrote:
Hi all,
I pushed today the data referencing feature for Eo objects in efl and
elementary. Check the commit log for explanations on why/how we do that.
From now, eo_data_get is deprecated, so please use eo_data_scope_get,
eo_data_ref or eo_data_xref instead.
Comments:
1. Super spank, you've included unrelated changes!!!
@@ -1240,7 +1244,7 @@ eo_xunref(Eo *obj_id, const Eo *ref_obj_id)
Eo_Xref_Node *xref = NULL;
EINA_INLIST_FOREACH(obj-xrefs, xref)
{
-if (xref-ref_obj == ref_obj)
+if (xref-ref_obj == ref_obj_id)
Fixes another issue that you've introduced (or so it seems).
2. I don't like the way you handle the data. You keep a refcount for it,
but you actually delete the data even if the reference is non-0. You
should do it like a true reference count.
You should free the data once the reference count hits 0.
You should always maintain at least one reference count (the object
referencing it's own data) so when the object is deleted that will get
decremented and the data will be freed or kept according to the
maintained reference count.
3. +_eo_data_xref_internal
You return uninitialised data if klass is NULL.
I know your code in it's current iteration won't get there, but still,
initialise it, or remove the klass!=NULL check if you think it should
never happen. At the moment it's unclear.
4. I don't understand how this even get in. You didn't add tests at all.
It's Eo, in Eo we actually test shit. I'm really tempted to just revert
it. Especially since my e got slow and annoying since your changes
(among others, haven't checked which).
Bottom line. Fix it. It needs fixing. Especially test stuff. Though the
general patch looks good and clean.
--
Tom.
--
Introducing AppDynamics Lite, a free troubleshooting tool for Java/.NET
Get 100% visibility into your production application - at no cost.
Code-level diagnostics for performance bottlenecks with 2% overhead
Download for free and get started troubleshooting in minutes.
http://p.sf.net/sfu/appdyn_d2d_ap1
___
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
Re: [E-devel] Eo object data referencing
On 05/01/2013 04:59 PM, Tom Hacohen wrote:
On 01/05/13 09:13, daniel.za...@samsung.com wrote:
Hi all,
I pushed today the data referencing feature for Eo objects in efl and
elementary. Check the commit log for explanations on why/how we do that.
From now, eo_data_get is deprecated, so please use eo_data_scope_get,
eo_data_ref or eo_data_xref instead.
Comments:
1. Super spank, you've included unrelated changes!!!
@@ -1240,7 +1244,7 @@ eo_xunref(Eo *obj_id, const Eo *ref_obj_id)
Eo_Xref_Node *xref = NULL;
EINA_INLIST_FOREACH(obj-xrefs, xref)
{
-if (xref-ref_obj == ref_obj)
+if (xref-ref_obj == ref_obj_id)
Fixes another issue that you've introduced (or so it seems).
Super spank on this!!! So yes, it's a fix of the Eo Id commit. And what?
Spank because it is on the same commit. For that, I call you enculeur
de mouches. Learn, it is a new french expression for you.
2. I don't like the way you handle the data. You keep a refcount for it,
but you actually delete the data even if the reference is non-0. You
should do it like a true reference count.
You should free the data once the reference count hits 0.
You should always maintain at least one reference count (the object
referencing it's own data) so when the object is deleted that will get
decremented and the data will be freed or kept according to the
maintained reference count.
I will check it. For the moment, EO_DEBUG is disabled (otherwise, we
will have to spank the entire world for the errors that will appear).
The most important was to push the new APIs for the release. The next
phase of the data referencing will focus on this part.
3. +_eo_data_xref_internal
You return uninitialised data if klass is NULL.
I know your code in it's current iteration won't get there, but still,
initialise it, or remove the klass!=NULL check if you think it should
never happen. At the moment it's unclear.
Even if we don't use it, I had to initialize it. I will fix it.
4. I don't understand how this even get in. You didn't add tests at all.
It's Eo, in Eo we actually test shit. I'm really tempted to just revert
it. Especially since my e got slow and annoying since your changes
(among others, haven't checked which).
So revert. I will not push it back. And before accusing me of making
your E slow, just check WHICH COMMITS ARE AROUND MINE!
Check, not the name but the contents of each.
Bottom line. Fix it. It needs fixing. Especially test stuff. Though the
general patch looks good and clean.
As usual, the good guy sentence!
--
Tom.
--
Introducing AppDynamics Lite, a free troubleshooting tool for Java/.NET
Get 100% visibility into your production application - at no cost.
Code-level diagnostics for performance bottlenecks with 2% overhead
Download for free and get started troubleshooting in minutes.
http://p.sf.net/sfu/appdyn_d2d_ap1
___
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
--
Introducing AppDynamics Lite, a free troubleshooting tool for Java/.NET
Get 100% visibility into your production application - at no cost.
Code-level diagnostics for performance bottlenecks with 2% overhead
Download for free and get started troubleshooting in minutes.
http://p.sf.net/sfu/appdyn_d2d_ap1
___
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
Re: [E-devel] Eo object data referencing
On 01/05/13 15:27, daniel.za...@samsung.com wrote:
On 05/01/2013 04:59 PM, Tom Hacohen wrote:
On 01/05/13 09:13, daniel.za...@samsung.com wrote:
Hi all,
I pushed today the data referencing feature for Eo objects in efl and
elementary. Check the commit log for explanations on why/how we do that.
From now, eo_data_get is deprecated, so please use eo_data_scope_get,
eo_data_ref or eo_data_xref instead.
Comments:
1. Super spank, you've included unrelated changes!!!
@@ -1240,7 +1244,7 @@ eo_xunref(Eo *obj_id, const Eo *ref_obj_id)
Eo_Xref_Node *xref = NULL;
EINA_INLIST_FOREACH(obj-xrefs, xref)
{
-if (xref-ref_obj == ref_obj)
+if (xref-ref_obj == ref_obj_id)
Fixes another issue that you've introduced (or so it seems).
Super spank on this!!! So yes, it's a fix of the Eo Id commit. And what?
Spank because it is on the same commit. For that, I call you enculeur
de mouches. Learn, it is a new french expression for you.
Dude, seriously that's *SO* not nit-picking. If there's anything
important about committing is this. People may want to cherry-pick
commits that fix stuff, but wouldn't want to introduce new issues.
People (me?) might have wanted to violently revert this commit. Would
have been really annoying had I had to commit the fix manually
afterwards. Really, if there's one important thing, it's this.
2. I don't like the way you handle the data. You keep a refcount for it,
but you actually delete the data even if the reference is non-0. You
should do it like a true reference count.
You should free the data once the reference count hits 0.
You should always maintain at least one reference count (the object
referencing it's own data) so when the object is deleted that will get
decremented and the data will be freed or kept according to the
maintained reference count.
I will check it. For the moment, EO_DEBUG is disabled (otherwise, we
will have to spank the entire world for the errors that will appear).
The most important was to push the new APIs for the release. The next
phase of the data referencing will focus on this part.
Has nothing to do with EO_DEBUG... It's just the count itself which
needs fixing. As you mentioned, the API is more important, nonetheless,
this should be fixed.
3. +_eo_data_xref_internal
You return uninitialised data if klass is NULL.
I know your code in it's current iteration won't get there, but still,
initialise it, or remove the klass!=NULL check if you think it should
never happen. At the moment it's unclear.
Even if we don't use it, I had to initialize it. I will fix it.
Cool.
4. I don't understand how this even get in. You didn't add tests at all.
It's Eo, in Eo we actually test shit. I'm really tempted to just revert
it. Especially since my e got slow and annoying since your changes
(among others, haven't checked which).
So revert. I will not push it back. And before accusing me of making
your E slow, just check WHICH COMMITS ARE AROUND MINE!
Check, not the name but the contents of each.
I didn't say it was you, and as I told you on IRC (or xmpp, can't
remember), I'm looking into that. It actually doesn't look like you.
That was just an additional comment, the most important part is the
inclusion of testing, which you have SERIOUSLY fucked up. As I've said,
Eo is CORE if Eo breaks, everything breaks. Eo should be tested. Well
everything should be tested, but Eo must be tested. Also, Eo is already
well tested, don't ruin that.
Bottom line. Fix it. It needs fixing. Especially test stuff. Though the
general patch looks good and clean.
As usual, the good guy sentence!
I'm a schizophrenic good-cop, bad-cop, you know that.
--
Tom.
--
Introducing AppDynamics Lite, a free troubleshooting tool for Java/.NET
Get 100% visibility into your production application - at no cost.
Code-level diagnostics for performance bottlenecks with 2% overhead
Download for free and get started troubleshooting in minutes.
http://p.sf.net/sfu/appdyn_d2d_ap1
___
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
