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

Reply via email to