On Thu, Jul 28, 2011 at 7:20 AM, Mike McCormack <mj.mccorm...@samsung.com> wrote: > On 07/27/2011 10:08 PM, Cedric BAIL wrote: >> So it will most of the time work, but in some racy case, not. Sounds >> to me like this doesn't change much from the current behaviour. I >> agree, it will work more often than previously, but still hidding bug >> until it's to late. > > It is a change it goal. The goal will be to make ecore calls thread safe, > and any non-thread safe behavior is a bug. > >> People should never use things they don't understand... and so few >> people understand threads. Just one question, do they use ecore_thread >> or there own stuff ? > > I disagree with "people should never use things they don't understand". > > The whole point of a library or any other programming abstraction is > to hide details and save people reimplementing these things over and over > themselves. > > Some developers in Samsung use ecore_thread, and some don't. Honestly, > ECORE_MAIN_LOOP_ASSERT has shown up quite a few problems with code. > Though the majority of developers understand that EFL code should be > single threaded, there are still many crashes due to threading issues. > I think EFL will be seen as "more stable" if it is thread safe... thus > these patches.
Maybe stable, but the rendered scene will not make sense and be highly difficult to debug. So spank and point people to the right solution. If you try to avoid segv by making code that are buggy almost working in most case, you are trying to hide issue and they will bite you harder when they come back. I think we should display error message that people do understand well and that point them to where they should fix their code and how they should fix it. And I am sure we can provide such a warning. >> You can't make it work sanely, how are you planning to synchronize the >> rendering state with your request from a thread. It's just not > > Let's stick to technical stuff rather than abstract ideas like sanity. > > I'm only talking about ecore here. Step by step... (Raster has big plans > for the rendering engine involving threads.) As far as I understand raster plan for rendering, it doesn't involve the user api of evas at all. It will continue to be a seriealised list of function call from the same main thread, then something will trigger evas_render and compute what is needed for doing the rendering asynchronously, before waking up all the rendering thread and getting out of evas_render and back to the application logique. And if you want you can put all this call in another thread, but it will always require to come from the same thread or they will never be any synchronization of the object state before entering the rendering state. > If the code is lacking in one area or the other, please point it out. > (Actually, some big bits, like locks around the idlers are missing from > the patch I sent.) See my answer to gustavo if you want something that work in all case. >> I clearly disagree on that patch going in (and I am still not >> discussing the issue of performance here). > > Well three points: > > * nobody complained when I added checks to show that ecore calls are coming > from the same thread, even though the performance "impact" would be > comparable. > (See ECORE_MAIN_LOOP_ASSERT) Because if needed, we can turn it off without breaking application or any behaviour. Something that was working with it on will work with it off. It something that we could use in a development build and remove from the production build without the need to worry about it at all. > * given that SMP is widely available these days, I would think that > enabling simple thread based programming could dramatically improve > performance of some code. Agreed, that's why we have Ecore_Thread in Ecore (stuff that I added) > * I will provide a way to switch off thread safety at build time That wouldn't help, because you will break application. There is no way to have a development build and a production build with that kind of feature, either you have only one application in your system and you know that it doesn't require Ecore to be thread safe and you can turn it off, or you just can't turn this feature off. > Right now my thoughts are: > > * support single main loop only > * add support for waking the main loop when timers, fds, etc. are added I thing that this will get confusing and people will introduce more bug, because the barrier with thread safety will be blured, some function call will work, some won't. At the end, I think you are increasing the complexity of the EFL without any benefit, you will have more and more complex code to debug (because every thing will not be thread safe). I really am more for a macro that display a spank with backtrace and do nothing in all efl function call (including evas, edje and elementary), that point to Ecore_Thread documentation. In fact halt of that code is already provided by eina, I can provide it quickly if that feat your need. This will put the burden of the fix on the developer and not you. They will know exactly where and how to fix it. -- Cedric BAIL ------------------------------------------------------------------------------ Got Input? Slashdot Needs You. Take our quick survey online. Come on, we don't ask for help often. Plus, you'll get a chance to win $100 to spend on ThinkGeek. http://p.sf.net/sfu/slashdot-survey _______________________________________________ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
