On Thu, Jun 19, 2008 at 1:57 PM, Cedric BAIL <[EMAIL PROTECTED]> wrote: > On Thu, Jun 19, 2008 at 6:48 PM, Gustavo Sverzut Barbieri > <[EMAIL PROTECTED]> wrote: >> On Thu, Jun 19, 2008 at 1:34 PM, Cedric BAIL <[EMAIL PROTECTED]> wrote: >>> On Thu, Jun 19, 2008 at 3:13 PM, Gustavo Sverzut Barbieri >>> <[EMAIL PROTECTED]> wrote: >>>> On Thu, Jun 19, 2008 at 6:15 AM, Cedric BAIL <[EMAIL PROTECTED]> wrote: >>>>> On Wed, Jun 18, 2008 at 7:44 PM, Gustavo Sverzut Barbieri >>>>> <[EMAIL PROTECTED]> wrote: >>>>>> On Wed, Jun 18, 2008 at 1:53 PM, Cedric BAIL <[EMAIL PROTECTED]> wrote: >>>>>>> Hi, >>>>>>> >>>>>>> Here is a patch that add support for background preloading of a data >>>>>>> image. You can now, if you know what you do, ask evas to load the data >>>>>>> of an image in memory before it is displayed (look at the sample code >>>>>>> for expedite). >>>>>>> >>>>>>> The code is quite simple, we have now a work queue where we add all >>>>>>> the Image_Entry we need to preload. They are processed one after the >>>>>>> other in another thread. When it's done, it notify evas main thread >>>>>>> with evas async API and work on the next Image_Entry. >>>>>>> >>>>>>> This means that we now require that engine surface creation and >>>>>>> loader should be thread safe. It's currently the case for all the >>>>>>> engine I know something about, so this should not break anything. >>>>>>> Anyway this code is optionnal and could be completely unactivated in a >>>>>>> per engine basis or globally. >>>>>>> >>>>>>> As always have fun reviewing this patch. >>>>>> >>>>>> so here it goes: >>>>>> >>>>>> configure.in: $build_async_preload should depend on async_events, no? >>>>> >>>>> You are right. >>>>> >>>>>> - EAPI Evas_Bool evas_async_events_put (void >>>>>> *target, >>>>>> Evas_Callback_Type type, void *event_info, void (*func)(void *target, >>>>>> Evas_Callback_Type type, void *event_info)); >>>>>> + EAPI Evas_Bool evas_async_events_put (const void >>>>>> *target, Evas_Callback_Type type, void *event_info, void (*func)(void >>>>>> *target, Evas_Callback_Type type, void *event_info)); >>>>> >>>>>> you made target const, but not in the callback... this is strange. >>>>> >>>>> Well in most place in the EFL, when a function doesn't free/modify an >>>>> object we mark it as const even if some outside fonction could modify >>>>> it. I don't have a hard opinion on this subject, so if you prefer I >>>>> will remove the const stuff. >>>>> >>>>>> As for the rest of the code. I told you at IRC, but you insist to not >>>>>> listen to me, so the review is: do not use mutex or cond, use a pipe! >>>>> >>>>> Lol ! Sorry but the code was already written, my problem was just >>>>> thread priority at that time :-) So I didn't want to rewrite it as I >>>>> already have something working. >>>>> >>>>> And regarding pipe, I just see one problem using them, we will not be >>>>> able to latter add the possibility to reprioritise the loading of >>>>> image. This could be usefull in the case of an image viewer I believe. >>>> >>>> You can even implement this, instead of reading and processing it >>>> directly, you read and append to some worker-only list, sorted by >>>> priority (another member of work-info), then you process in that order >>>> and you pipe back to main thread as usual... still no locks, each >>>> thread is safe, etc... >>>> >>>> >>>>> I also don't understand why you dislike so much mutex and cond stuff. >>>> >>>> because they're useless in this case. We should not have to have a >>>> critical section where just one needs to operate. Each thread should >>>> have its own list/queue, just the main thread should allocate/free the >>>> work-info pointer. >>> >>> I disagree, you will always need a lock inside >>> evas_cache_image_load_data. Or you will have a race condition with the >>> loader itself. If you are unlucky, you could have just decided to >>> start loading a image, but you don't have yet the time to change the >>> state to PROCESS. So in the main evas thread, you cancel it and you >>> start to load the data, in the worker thread, you just change to >>> PROCESS and you start to load the data. This end-up, in the best case, >>> with just some memory leak. So you need in the critical path to check >>> this. It's why I did put mine and it's not locking if you don't have a >>> worker thread in my case. >>> >>> I don't see how I can go around that without lock. >> >> You shouldn't have that case, because if it's loading on the thread, >> makes no sense to load on the main thread. So if you want to handle >> such thing, you can have atomic operations on such flag. > > Preloading is done in the background when requested. But loading stay > in the main thread, we don't want to delay and slow down all call to > image_load_data (remember it's in the critical path of all call to > image_draw, and image_draw of all engine assume that the data are > ready after this call).
Which slow down would be this? You need to check if it's loading anyway. this is an atomic operation on a flag. If it's being processed, then you wait there, otherwise you just flag it as canceled. -- Gustavo Sverzut Barbieri http://profusion.mobi embedded systems -------------------------------------- MSN: [EMAIL PROTECTED] Skype: gsbarbieri Mobile: +55 (19) 9225-2202 ------------------------------------------------------------------------- Check out the new SourceForge.net Marketplace. It's the best place to buy or sell services for just about anything Open Source. http://sourceforge.net/services/buy/index.php _______________________________________________ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
