On Wed, 28 Feb 2018 15:30:24 +0000 Mike Blumenkrantz <[email protected]> said:
> 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. i'll put up something there to describe what i think is a clean design for this that will work well with minimal trouble. i've actually been implementing efl.thread too. i ended up making an efl.appthread parent class that specifically is for non-mainloop threads, where efl.app is the main loop thread. i have threads (parent loop and child threads at least) 2-way communicating right now etc. i need to fix the efl.io on efl.app so it goes to stdin/out for efl.exe to mirror efl.thread behaviour. then i will take a look at the tests you added and adjust them or re-add them back. as i said - i've been holding off on tests until things settle. my test code i have to alter regularly to fit and its not a digestible automated test (it printfs so i know whats going on). > 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 > -- ------------- 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
