It seems like there's strong opinions here, and since this is something which potentially matters to all future EFL development I would propose that instead of arguing about this on the list for another X number of weeks both of you write up the key points (pros and cons, code snippets) of your side on a wiki page such as https://phab.enlightenment.org/w/efl_app/ . This will make it easier for everyone to make an informed decision and will further allow us to reuse the information in documentation later on if anyone ever questions why it was done in this way.
On Wed, Feb 28, 2018 at 12:35 AM Carsten Haitzler <[email protected]> wrote: > On Tue, 27 Feb 2018 13:12:59 -0500 Cedric Bail <[email protected]> said: > > wo.. a cedric! long time... > > > For whatever reason, I didn't receive the first email of this chain, so > will > > answer here. > > > > -------- Original Message -------- > > On February 27, 2018 7:21 AM, Mike Blumenkrantz > > <[email protected]> wrote: > > >You've certainly raised some valid points which bear further looking > into, > > > thanks! > > > > > > At a minimum, this patch was intended to introduce a base > implementation of > > > a singleton "app" object which could (and should) have tests written > for it > > > as it's improved upon. If you or others want to do that work feel free, > > > I've moved on to other items for the near future. > > > > > > On Tue, Feb 27, 2018 at 12:07 AM Carsten Haitzler [email protected] > > >wrote: > > > > > >>On Mon, 26 Feb 2018 11:03:03 -0800 Mike Blumenkrantz > > >>[email protected] said: > > >>this change has a fair few problems to it... i don't think this is the > > >> right > > >> way to go. > > >>this change from efl_main_loop_get() doesn't make sense > > >> - job = efl_add(EFL_IO_MANAGER_CLASS, efl_main_loop_get()); > > >> > > >> - job = efl_add(EFL_IO_MANAGER_CLASS, > > >> efl_app_main_loop_get(efl_app_get())); > > > > I disagree on both of those line. You get the main loop when your > application > > start and all your object do get you there. The scenario in which you > need to > > directly access the main loop should be really limited (tests and legacy > code > > are what come to my mind). > > i literally have test code that does: > > if (loop == efl_main_loop_get()) { } else { } > > i can't do that anymore from threads. the loop does not have an EO parent > set > to the efl app object, so i cant get the parent of my loop and do an isa on > it... that's why i say... this doesn't make sense... it also doesn't make > sense > to just make it longer. > > > >>you can't access the app object from anywhere other than the main loop > > >> thread because it's a thread local object (the default)... so all this > > >> does it make this longer for no benefit. > > > > This is the point. Why would you need to access this object from another > > thread ? > > you can't know if your loop is the main loop or not with this scheme. if > you > use superclassing then isa() can tell you. if it actually used eo parent > objects, then getting parent could tell you, with the exception that the > parent > is thread local so this cant work. if it was shared you cross domains too. > > it's better to superclass here than use parent/child. > > > >> if can't be a shared object either because you also do things like: > > >> - EINA_LIST_FOREACH_SAFE(pd->loops, l, ll, loop) > > >> - efl_del(loop); > > >> > > >>you want to have multiple loops in a single thread-local shared > object? you > > >> can't mix shared and non-share objects. > > > > There is a lot in this sentence. The fact we can't mix shared and > non-shared > > object is going to be a serious problem with our parent model and object > > refcounting in general. Basically we will require to have another tree of > > shared object on the side that is not connected to the one that are not > > shared. And all of this object have to be controlled by the user call to > the > > domain API. This is a lot of receipe for error and problems. > > correct. it's just not practical to mix objects that are locked and > protected > with ones that have no locking on them. delete a shared obj and it tires to > delete non-0shared children. BOOM. the children aren't protected with locks > etc. ... there are good reasons not to allow the "streams to cross". and > they > have nothing to do with the shared objects that provide locking for you, > but to > do with locked vs not-locked objects/data interacting in general > > > >> this wouldn't even be able to delete the other loops belong to > threads, > > >> and multiple loops in a single thread just make zero sense.loops are > > >> intended to run a thread, not be multiple of them within a thread. i > > >> thought this had been discussed? > > > > It has been and the conclusion was that the new logic that was pushed in > tree > > broke the previous one which had the intent to make it work. We > concluded it > > was hard to get that working again and not a priority, but not that it > should > > not be a scenario for the future as much as I remember. My expectation > with > > this code is that only the main loop running in the main thread end up > there. > > Overall, this is a details as this new thread model is really not a > priority > > for this release as no bindings can use it and legacy API can still be > used. > > i think it's important to set the base here because of exactly the above. i > remember distinctly discussing face to face in person "1 loop per thread" > with > you. never was there any talk about "let's have multiple loops within a > thread > and somehow have them work together". i see no value in this. it just > artificially divides up a loop and adds complexity to making them all work. > > but if that list of loops is for loops across threads, my point stands - it > can't work for logical , not implementation reasons (unlocked objects being > nuked from another thread while they may still be running...) > > > <snip> > > > > >>here is what i have been mulling and i think might be right: > > >>an efl.app or efl.mainloop class that INHERITS from efl.loop. i.e.: > > >>class Efl.App (Efl.Loop) > > >> { > > >> ... > > >> } > > >>so the main loop will be of this efl.app or efl.mainloop class and then > > >> other loops in threads will be of the regular efl.loop class, not the > > >> efl.app class. no parent + child containing all the loops here and so > no > > >> cross-thread boundaries are crossed. then the efl.app or efl.mainloop > > >> class can have the signal events move to it. the same main loop or app > > >> object we had before stays, but it just gets a new class on top. > > > > We did consider that too, but we rulled against it as we think it will > likely > > increase maintenance complexity. We can disagree on that, but overall I > don't > > see why one would be easier than the other. > > i think the exact opposite... :/ > > > >> it's simpler and won't have the above issues (and others i can begin > to > > >> see/imagining). you know if your loop is the main loop with > efl_isa(loop, > > >> EFL_APP_CLASS) on it. the efl_main_loop_get() can change to > efl_app_get() > > >> and be shorter and sweeter, with perhaps later an > efl_current_loop_get() > > >> that gets the loop for that thread from a TLS that works in threads > and in > > >> main loop too once we have api to spawn threads and handle binding > them > > >> together with comms pipes. > > > > I am not a fan at all of relying on TLS for this things especially when > all > > the code will be given a context via at least an object that can lead you > > back directly to a loop or the application. > > the TLS is a convenience to get your thread's loop without having to find > it > via a parent object. that's all. it's an option. it isn't NEEDED as you get > passed your loop anyway (well will - via the args event callback that is > basically your init func for a thread). > > > >> it's simpler than your changes here and has fewer problems. i don't > think > > >> loops will or should be children like you have above of an object. > they > > >> have to be loosely coupled. they need to have just regular structs > ptrs > > >> inside the loop object to track child threads and how to communicate > with > > >> them. to delete them you need to co-operatively request their delete > e.g. > > >> via a pipe/fd setup to the other end, then the loop at the other end > exits > > >> automatically when it gets this event/request on their end of the > pipe and > > >> deletes itself. listen to the del event on the loop object to handle > > >> shutdown of anything pending on that thread if you want to ensure you > > >> clean up anything you created on the thread end of things. actually > any > > >> thread/loop probably has to wait for "child thread/loops" to exit > before > > >> exiting itself (should join all the thread's at a minimum). it can > hold > > >> the exit request pending and keep the loop running until all children > have > > >> reported back to have exited. this does present one possible > exception - > > >> the main loop. should it wait too? that is a good question, but > definitely > > >> all other threads/loops should wait so we end up with a nice organized > > >> tree of threads/loops or ... tasks that have "children" > (thread/process > > >> thread/task children not object children) finish before the parent > does so > > >> you have a nice guaranteed cleanliness by default at least. > > > > I think we are getting seriously distracted with all of this > threads/loops > > here. As said above, it is unlikely that we can make that part of our API > > stable for next release. It would require change in Eo and Eoliand to > make > > binding aware of thread ownership. This would obviously require to make > sure > > we need this for c and c++. apparently davemds needs the efl.exe and > related > classes too for python because python doesn't offer a loop integrated async > stdin/out io to a child executable. > > threads are just extending that python need into what is needed for c/c++ > anyway. it cleans up a lot and makes threads far nicer to use with > symmetric > i/o to/from a parent as opposed to one-way etc. > > i;m doing this because we need to at least set a baseline on design, > without > which bad things happen that put roadblocks in for the future. we don't > have to > implement everything. i don't plan to implement anything more than > "stdin/out" > style byte streams via the reader/writer interfaces. we could add a lot of > controls like suspend and resume of a thread from its parent etc. ... but > no > need right now. > > > that language that do have a very different approach to this problematic > be > > able to get the information they need to make binding work. With that > > constraint in mind, I am pretty certain we have no time to investigate > this > > details for next release and that the only case that matter in the use > case > > for one main loop with the goal of a simpler code for us to maintain > also. > > certainly for c/c++ this absolutely does not apply. :) > > > If you think it is best to have one object Efl.App that inherit from > Efl.Loop > > and that it will be easy to maintain. Please go ahead and do it. I have > no > > i have done it. locally. :) > > > strong opinion on this at all, but please keep in mind that we want to > make a > > release rather sooner than later and going down the threads rabbit hole > is > > not really something we have time to work on at this point. > > well i'm working on ecore because it basically wasn't being worked on... > and i > think a baseline here really matters to make it work well and set > guidelines > for direction etc. ... > > > Cedric > > > > > ------------------------------------------------------------------------------ > > Check out the vibrant tech community on one of the world's most > > engaging tech sites, Slashdot.org! http://sdm.link/slashdot > > _______________________________________________ > > 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] > > > > ------------------------------------------------------------------------------ > Check out the vibrant tech community on one of the world's most > engaging tech sites, Slashdot.org! http://sdm.link/slashdot > _______________________________________________ > enlightenment-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/enlightenment-devel > ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ enlightenment-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
