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). -- Cedric BAIL ------------------------------------------------------------------------- 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
