On Thu, 5 Sep 2019 18:29:53 +0200 Marcel Hollerbach <[email protected]> said:
> This mail seems to cover up 2 quite unrelated things: a bug (fix), and > the general discussion about efl-interfaces. the bug is just a side effect of being a little "careless" about going around and changing api's and their internals and missing behaviors that have been depended on (you can file_set to another file/group or the same and your swallowed/packed objects follow along unless a new group is missing that and then the child is orphaned - that's what used to happen). it's what caused me to go digging and it was just a call to focus on not breaking such things in the process of rejigging a whole chunk of api's. > The bug you have fixed in EFL is pretty much unrelated to eo or the file > interface. It was added in f10a3c9ee36cd270045f7e30fd3716ef15e3106d > which is not related to the work that was put into the file interface in > the last release. > > I also have to admit here that i cannot fully agree with your reasoning > for 5 instead of 1 eo call. efl_file_simple_load is not a eo call. ok - it's 4 then. it was masquerading to look like an efl eo api call. still 4x as many at that point > Additionally, file_set key_set and load are executed right after each > other, which makes use of the eo_id cache here, which only leaves the > function pointer lookup, not the pointer lookup, which cuts off a lot of actually the function lookup is also expensive :( > cost. The unload here is a special case which is happening here in edje > itself (prior to your commit, not anymore), not every object does call > that. Additionally, normal usage of that API, outside the legacy > wrappers and outside edje, will probebly only result in file_set and load. well edje is one of the big users - one of the biggest. almost every image loaded also comes from the edj file so uses file+key too. so now it's 3 calls at least to set file, set key then load. 3 calls that used to be 1. i think that in trying to simplify in one way this has become worse in others. even with a single file (like path to a png) it's still 2 calls instead of one. if you had a key set like you loaded an image from an eet file (edj file) then u just set the file next and forget to set key to null an the next file type interprets keys differently (generic loader uses keys as timestamps to load from video files ....) then you have easier mistakes. i'm dubious if this is an improvement and i think this would have been better as just an optional key arg in some langs and express it that way. maybe change from file_set(file, key) to file_load(file, key) if we don't want it to be confused with a property... > I also tend to agree with you that we need to be carefull with what we > are doing via eo calls, and what can be combined, however, the file set > API is not a good example to showcase that, as this API most of the time i think it's a good example because of the above. it IMHO has not become better... it's at best just changed for a different set of issues and at worst become slower/more cumbersome/easier to get wrong. :) > results in a heavy operation performed afterwards, so the cost you pay > after calling that API is a lot higher than the cost calling the API > itself. Comparing that for example to our writer / reader API in eo, if > you often need to know if something is readable now you will all the > time call the same API over and over again, just to query a time little > boolean flag, which gets set by some foreign entity. For me comparing > those two examples, efl.file makes a lot more sense than for example > Efl.Io.Reader/Writer. it's and example of not considering performance in design. both of them are. the io one is of course worse. especially when you have lots of small reads or writes and you are going in and out of eo api a LOT. > Additionally, there is another cost of eo that we cannot really cut off, > like the cost we are paying for moving a object. A little introduction: > If you move a layout via efl_gfx_entity_geometry_set you will actaully call: > (On the widget object) > - canvas_object (geometry_set gets translated to position_set) > - widget (position_set) > - canvas_object again (position_set) > In the widget layer there is another call with geometry_set on the > canvas object which calls: > - canvas_layout > - canvas_group > - canvas_object yes. this is indeed another issue. to a large extent it already existed due to smart objects too having to move their children (we defer it until render time so we only do it once per frame and otherwise mark a smart obj dirty) and if we keep our object set down to just the visible set of objects then we're not doing as badly. i should have done just true nesting and relative coords long ago but i went for a flat model and that's my fault. the way around it is to just not have objects you don't see move or resize at all. have them hidden and stashed away or have them not even exist. > So in this one call we have 6 lookups where we are looking up, the real > eo_id, the func-ptr, and the pd. However, the reasoning for eo as a > security layer there is a little bit weak, we are in inheritance, we in general it's a security layer (as well as passing in things like super call flags via the eoid in bit flags etc.) INSIDE efl we could/should skip calling eo calls in many cases if we can if we don't need to. :) > KNOW that the object we are calling on is valid. Additionally, we can > calc the pd pointer by adding the diff between the two classes to the > next inheritance step, so all the work that is currently done by eo can > be basically saved at compile time. The methods for applying that to the > efl tree are not there yet, that needs more work, I would also love to > serve here numbers, but I had other things to do. that is true. it's something else to consider when writing code. can you bypass an eo call and call an internal function directly when writing code. :) > I am not aware of any interface where we said goodbye to performance for > the sake of clean and nice API. So as a example, the new Grid and List efl.io. :) to a small extent the file_set now. looking at the efl model now has a whole reflection thing with eina values and lots of extra code filling up efl. :) > widget have a common interface for doing placement, that saves us a lot > of duplicated code. The interfaces are designed in a way that eo funcs > are called as few as possible, so the handing over of the items etc. are > only involving eo for batching (like give me now 100 items starting > *there*), which means, we have 1-3 eo calls per placement of items for > "managing" the items, the placement itself then of course does not get > around the fact of calling geometry_set, which involves calling eo api. > The setup for all this also went quite a few rounds in terms of > developing it, and at any time performance was a reason for this, so I > am wondering a little bit where 1) and 2) and 3) are coming from. > > Additionally, I would be interested in 5) I do not see damage right now, efl.io. file_set+key_set. talking about the api > yes, you found a bug in a revision, but I guess that would happen with > or without eo and with or without the interface work. The tradeoff you > are making here is something I have not seen so far, there are > interfaces which surely did this, but those are beta, and they are also > kind of out of the view for now, as (speaking for me here) i am focusing > on the new widgets here, and I have not seen that much work going into > something outside of that (beside Xavi's work on improving docs just > *everywhere*) > > All I can say about that: there is the efl: api board, where there are > tickets for each class/interface, so we can at least try to keep an > overview of the insane amount of classes and interfaces we have, the > items there are currently something to look into, the Efl.Io things are > *not* in there. And I cannot locate any class that is in the stabilized > column that has problems with the purity over performance thing brought > up here :) > > Greetings, > bu5hm4n > > On 9/5/19 2:45 PM, Carsten Haitzler (The Rasterman) wrote: > > I'm noticing a bit of a trend of what I might call "endless fiddling > towards > > perceived perfection". This fiddling is adding cost/overhead, making > things > > slower and adding bugs. I do not intend to point fingers here so do > NOT take > > this as "this person is at fault" or personally as the issue is more > one of a > > trend and almost everyone seems to be on the bandwagon. > > > > For example, the changes from file_set that take a file + group now > set them > > separately as 2 keys. Like now edje_file_set() does: > > > > efl_file_unload() <- this breaks things btw and adds a bug > > efl_file_simple_load() > > efl_file_set() > > efl_file_key_set() > > efl_file_load() > > > > This has gone from 1 eo call that it was a while ago to a file_set to > 5 of them > > now as it has to get the values via eo API etc. and added breaks in > behavior. > > That's 5x the EO call cost added there to the same functionality. I > probably > > have missed some more calls I'd find if I dig enough. > > > > Eo API is expensive. It adds a safety layer and call abstraction that > isn't > > free. There has been a lot of work to try and shave down its cost and > it's > > unlikely to get staggeringly faster any time soon, if ever. Certainly not > > unless major effort is put to profiling it and making carefully > crafted memory > > layouts and specific architecture optimizations. > > > > Everyone should be designing APIs to go through the EO call API as > *LITTLE AS > > POSSIBLE*. That means things like file_set should not have split up > file and > > key. More objects are also costly. Events/callbacks are not cheap > either. This > > means probably rolling more into a single API call, single object or > single > > event and doing these calls less and always considering design from > THAT point > > of view. This may not be perfect but it's practical and performant. > > > > There is a limit to how much you can merge of course, but the above > split of > > file and key is a fairly pointless split (key can always just be NULL > or some > > bindings/langs can tag it as an optional argument and C can just use > NULL to > > indicate it isn't there). Also consider how often the API is called. The > > file_set()s are called a lot during startup/setup of a > window/app/object and > > sub objects. It's not good to go add costs to things that have lots > of calls > > already going on. > > > > I brought up the whole efl.io interface design a few weeks ago too > (on my todo > > list to go redo) with it also going in and out a lot and being costly. > > > > Can we please take a few big steps back and: > > > > 1. Stop the fiddling around the edges hunting for perfect design but > > forgetting performance. It's busy work too that endlessly delays a > stable EO > > API. If it doesn't make high impact better design, then perhaps don't > do it? > > 2. When something is done, stop thinking about perfect API design and > consider > > the costs. That is life and design has to work around it. There is > already a > > mountain of this peppered throughout eo API and it's not good. > > 3. If you are going to try a whole new design then try it in > isolation as an > > experiment that isn't beneath existing API and so it won't break > things or have > > major performance impacts. > > 4. Can we bring these kinds of design change discussions to the > mailing list? > > Like splitting up file and key IMHO is a fairly big change in design > that has > > impacts all over like above and changes a fairly large API design > premise that > > has been with EFL for well over a decade. > > 5. Start undoing some of the damage done. > > 6. Looking for ways to improve EO API not in terms of purity but in > terms of > > performance? > > > > :) > > > > > _______________________________________________ > enlightenment-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > -- ------------- Codito, ergo sum - "I code, therefore I am" -------------- Carsten Haitzler - [email protected] _______________________________________________ enlightenment-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
