[prev in list] [next in list] [prev in thread] [next in thread] 

List:       enlightenment-devel
Subject:    Re: [E-devel] [EGIT] [core/efl] master 02/06: eo: add support for restartable event inside nested ca
From:       Tom Hacohen <tom () osg ! samsung ! com>
Date:       2016-05-24 8:54:14
Message-ID: cf108372-6d43-7fbc-c1a6-95b273fd9875 () osg ! samsung ! com
[Download RAW message or body]

On 23/05/16 11:02, Cedric BAIL wrote:
> Le 23 mai 2016 10:35 AM, "Tom Hacohen" <tom@osg.samsung.com> a écrit :
>>
>> On 20/05/16 17:48, Cedric BAIL wrote:
>>> On Fri, May 20, 2016 at 9:21 AM, Cedric BAIL <cedric.bail@free.fr>
> wrote:
>>>> On Fri, May 20, 2016 at 8:36 AM, Tom Hacohen <tom@osg.samsung.com>
> wrote:
>>>>> We kind of talked about it in person, but I would like to raise this
>>>>> again here because I don't think this topic was concluded.
>>>>>
>>>>> I don't like this and I don't think it's necessary.
>>>>>
>>>>> You are polluting all of Eo for something that could easily be a
>>>>> tailored fix for where you need it, which is essentially mainloop.
>>>>>
>>>>> Eo is OOP, you can override callback call/callback add/callback del
> and
>>>>> etc. to specifically deal with these kind of setups. You are adding a
>>>>> bool to all the events (not a big deal) and adding a pointer to all of
>>>>> the objects (more of a big deal), you're polluting the eo code (also a
>>>>> big deal) and all for what? For an activity that is only ever used in
>>>>> mainloop just because of mainloop_iterate? Implement it in the
> mainloop.
>>>>> This will not expose new API but will work exactly the same.
>>>>>
>>>>> So again, why make this change? Please do as recommended above and rid
>>>>> us of this. :)
>>>>
>>>> As said in person, I disagree. There is for sure other case where we
>>>> are at risk of infinite event being triggered and the only way to
>>>> prevent this from happening is if they are marked as @restart. If you
>>>> prefer another name, not a big deal, but in general having the
>>>> infrastructure to handle it is most likely going to be useful. @hot
>>>> wasn't much used until we started noticing bugs and fixed them as we
>>>> go. We are still into a trial and error experiment with most of our
>>>> events. So are you sure there is no other case where an infinite
>>>> number of event wouldn't be triggered ?
>>>
>>> Ah, and I forgot to talk about the optimization. I think it is not a
>>> real concern. We can always use Eina_Cow to group things together when
>>> it start to matter. We could for example move the del intercept into
>>> the object itself (By the way, why is it not an object function ?) and
>>> maybe even group it with children as most likely the mojority of our
>>> object are leaf object. Also all of them could already be moved to the
>>> extention area and we could make the extention an Eina_Cow to reduce
>>> code complexity. So I still don't think it really matters.
>>>
>>
>> We can use Eina_Cow (also comes with a cost), but we can use this
>> regardless. Still though, the fact that we can maybe improve things
>> doesn't mean it should be API... You are also adding to the event
>> structures and making them more complex in addition to adding the extra
>> macros and making the API more complex. You still haven't addressed the
>> core argument. You are saying you can make this less bad, but less bad
>> is still worse than my proposed alternative.
>
> Well, you can see the problem the other way around. I will have to fully
> reimplement the callback infrastructure (100% code copy wherever that
> single feature is needed). I fail to see why that would be better than the
> current situation, especially if the final overhead is not noticeable and
> any alternative lead to a more complex and more difficult to maintain code
> base. I absolutely don't see any benefit in moving that to another class.
> it will also require additional change to eolian as it should not generate
> this events. Really a lot more code and complexity.
>
> I absolutely don't see any improvement with your proposition especially for
> something that can be useful for anyone that use events as a queue.

Have you benchmarked the overhead that you're saying that? It's one of 
the hottest path in the efl. I drastically optimised it once by removing 
an if. This is an empty claim, show some data!

Not another class, just the implementation somewhere else.

It's not complexity in Eolian because you're not gonna make this generic 
solution, but a tailored one, so Eolian is not even aware. I'm happy to 
implement it myself if you don't want it.

It's not a code copy. This calling path is completely different and a 
completely different calling mechanism. Yes, we'll probably copy the 
"for" part, but the content will be different...

>
>> Del intercept: off topic for this thread, though no idea, I didn't touch
> it.
>
> Ok, something to investigate I think.
>
> Cedric
>
>> --
>> Tom.
>>
>>
> ------------------------------------------------------------------------------
>> Mobile security can be enabling, not merely restricting. Employees who
>> bring their own devices (BYOD) to work are irked by the imposition of MDM
>> restrictions. Mobile Device Manager Plus allows you to control only the
>> apps on BYO-devices by containerizing them, leaving personal data
> untouched!
>> https://ad.doubleclick.net/ddm/clk/304595813;131938128;j
>> _______________________________________________
>> enlightenment-devel mailing list
>> enlightenment-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
>>
> ------------------------------------------------------------------------------
> Mobile security can be enabling, not merely restricting. Employees who
> bring their own devices (BYOD) to work are irked by the imposition of MDM
> restrictions. Mobile Device Manager Plus allows you to control only the
> apps on BYO-devices by containerizing them, leaving personal data untouched!
> https://ad.doubleclick.net/ddm/clk/304595813;131938128;j
> _______________________________________________
> enlightenment-devel mailing list
> enlightenment-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
>


------------------------------------------------------------------------------
Mobile security can be enabling, not merely restricting. Employees who
bring their own devices (BYOD) to work are irked by the imposition of MDM
restrictions. Mobile Device Manager Plus allows you to control only the
apps on BYO-devices by containerizing them, leaving personal data untouched!
https://ad.doubleclick.net/ddm/clk/304595813;131938128;j
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

[prev in list] [next in list] [prev in thread] [next in thread]