Re: [E-devel] [RFC] Async preload of image data

[Gustavo S. B. <bar...@pr...>]

On Tue, Jun 24, 2008 at 1:46 PM, Jose Gonzalez <jos...@ju...> wrote:
>
>>>>   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.
>>>>
>>> hmmm - i like the idea - yes. missing cache surface alloc mutex :( that should
>>> not be hard to add - a little bit of extra code. also missing Evas.h prototype
>>> for exporting the preload call. looks like most engines have the calls
>>> supported. the problem is what to do out fd'with the locks on alloc. as such it
>>> is unlikely to cause problems as it stands in the patch - but may if the alloc
>>> happens while deleting/freeing an image. the images themselves all need locks
>>> now for anything accessing them.
>>>
>>> for other comments. i dont think we need to worry about fd's - the async fd is
>>> wrapped already. we dont need fd's internally in the async loader backend as it
>>> is by nature thread-aware and thus can and will use pthread primtitives and can
>>> use them for internal syncronisation or anything it likes. look at the evas
>>> pipe thread code - it doesnt use any fd's because it doesnt need to use them.
>>> there is no NEED for it to wake up select. i dont think u need to worry about
>>> this. we need to worry about the locks now needed on all images as the surface
>>> alloc is async to the mainloop. mainloop may do other things (like begin to use
>>> the image for other things while the async thread is allocing and hasnt been
>>> cancelled yet). the safe solution as above is locks on all images - but this is
>>> kind of ugly. i'd rather have this be less invasive in this department. but
>>> this means all operations on images need a mutex lock/unlock. :(
>>>
>>
>> I strongly disagree, see my examples, using that will make your life
>> much easier. The idea is very simple, the worker thread doesn't even
>> need a select, it can just sleep on read() and that's it. Ok, you can
>> do that with pthread_* primitives, but I don't see why. You get images
>> to process, you send them to the thread, that keeps a "to do" list,
>> process by priority order. These images (actually the structure that
>> points to it) will be marked with the priority and the state (todo,
>> done, canceled, ...) and it will all work well without any locks.
>> Summary: worker thread just look at some flag, if it's PROCESS then
>> put the pixels, notify back. No image allocation/free should be done
>> on the worker thread, thus no locks.
>>
>>
>> Also, giving the use case, I'd avoid locks on images and alloc them on
>> the main thread before requesting it to be loaded, this way no pointer
>> will change, just the contents of it, then no problem. This will make
>> life much easier for a feature that is just aimed to make application
>> not block while reads huge images from slow access memory (disk),
>> avoiding complications on the existing code paths.
>>
>
>      Gustavo has a point here. This kind of thing can easily get out of hand
> in complexity and obscurity.. for what kinds of uses, effects, ....?
>      I wonder if it might not be 'better' to have such (sync and/or async)
> image loading done by a separate server which would fill or hand out image data
> via some ipc mechanism? Then the server can do quite few more things than even
> evas does now such as take full paths into account, monitor src file changes,
> give better file info, error reporting, progress reporting, etc. And it can
> implement things via whatever means might be best, and possibly be useful to
> many other apps/libs as well.

This is out of Evas scope, at least this scope. You can implement such
things for things that are usually shared among lots of apps (ie:
Open/Save/Close icons), just have the main app to do that and share
the memory (shmem or equivalent) and set the pointer of evas image
data.

-- 
Gustavo Sverzut Barbieri
http://profusion.mobi embedded systems
--------------------------------------
MSN: bar...@gm...
Skype: gsbarbieri
Mobile: +55 (19) 9225-2202