Re: [E-devel] [RFC] Async events API
On Thu, Apr 24, 2008 at 11:15 PM, Gustavo Sverzut Barbieri [EMAIL PROTECTED] wrote: On Thu, Apr 24, 2008 at 1:57 PM, Cedric BAIL [EMAIL PROTECTED] wrote: On Wed, Apr 23, 2008 at 4:45 PM, The Rasterman Carsten Haitzler [EMAIL PROTECTED] wrote: On Tue, 15 Apr 2008 13:15:13 -0300 Gustavo Sverzut Barbieri [EMAIL PROTECTED] babbled: For simplicity, I would just process one message per callback from ecore_fd_main... but we can also use a poll/select inside this function and do what you want, without the need to fcntl to NONBLOCKING. just read a buffer of messages - if we don't get a complete message, store the partial one and keep it until the next call to process events then complete the message fetch. Ok. Here is an updated patch. The fd is still nonblocking and on partial read, it should be able to just restart cleany at a later time. You will now pass a function pointer so that you can call whatever kind of code you want (I doubt we really need something else than evas_object_event_callback). I also make it optional. Some more comments ? configure: s/build_async/build_async_events/g; make it default on (use auto to check for pthread, yes would fail without pthread and no is... well no). +static pthread_mutex_t _mutex; i think it's safer to just initialize the mutex here using (and remove the destroy call): static pthread_mutex_t _mutex = PTHREAD_MUTEX_INITIALIZER; evas_async_events_process(void): + if (_fd_read != -1) + do reverse this logic: if (_d_read == -1) return 0; this will avoid the unnecessary nesting and would remove the bug that it have today: check might be uninitialized in this function. Actually write the whole function like (more beautiful imho): if (_fd_read == -1) return -1; count = 0; while ((check = read(_fd_read, ((char *)current) + size, sizeof(current) - size)) 0) { size += check; if (size sizeof(current)) continue; if (current.func) current.func(current.target, current.type, current.event_info); size = 0; count++; } if (check 0) { ...; _fd_read = -1; } return count; For evas_async_events_put, also reverse the logic, I'd write: if (!func) return 0; if (_fd_write == -1) return 0; ... pthread_mutex_lock(_mutex); do { ... } while ( ... ) pthread_mutex_unlock(_mutex); this would not do unneeded locks and avoid nested code. Also, note that the function return Evas_Bool but in the version without BUILD_ASYNC_EVENTS you return -1. Aside these minor, mostly cosmetic, things, it's ready for inclusion :-) Thanks a lot for taking the time to do the review. Here is a final patch, and if nobody oppose, I will commit it during next week. -- Cedric BAIL diff --git a/configure.in b/configure.in index a08df81..775394f 100644 --- a/configure.in +++ b/configure.in @@ -1129,13 +1129,17 @@ AM_CONDITIONAL(BUILD_LOADER_PMAPS, test x$have_pmaps = xyes) pthread_cflags= pthread_libs= build_pthreads=no +has_pthreads=no # sched_getaffinity pthread_attr_setaffinity_np AC_CHECK_HEADERS(pthread.h sched.h, [ AC_CHECK_LIB(pthread, pthread_attr_setaffinity_np, [ AC_CHECK_LIB(pthread, pthread_barrier_wait, - [ build_pthreads=yes ], + [ + build_pthreads=yes + has_pthreads=yes + ], [ build_pthreads=no ] ) ], @@ -1173,6 +1177,26 @@ AC_ARG_ENABLE(pthreads, ) ### +## Async events +build_async_events=auto +AC_MSG_CHECKING(whether to build Async Events support) +AC_ARG_ENABLE(async-events, + AC_HELP_STRING([--enable-async-events], [enable async events support]), + [ build_async_events=$enableval ] +) +AC_MSG_RESULT($build_async_events) + +AC_MSG_CHECKING(whether we can build Async Events support) +if test \( x$build_async_events = xyes -o x$build_async_events = xauto \) -a x$has_pthreads = xyes; then + AC_MSG_RESULT(yes) + AC_DEFINE(BUILD_ASYNC_EVENTS, 1, [Build async events support]) + build_async_events=yes +else + AC_MSG_RESULT(no) + build_async_events=no +fi + +### ## MMX build_cpu_mmx=no case $host_cpu in @@ -1760,6 +1784,8 @@ echo SSE.: $build_cpu_sse echo ALTIVEC.: $build_cpu_altivec echo Thread Support..: $build_pthreads echo +echo Async Events..: $build_async_events +echo echo ARGB Software Engine Options: echo Sampling Scaler.: $scaler_sample echo Smooth Scaler...: $scaler_smooth diff --git a/src/lib/Evas.h b/src/lib/Evas.h index 22fb29f..18ba377 100644 --- a/src/lib/Evas.h +++ b/src/lib/Evas.h @@ -823,6 +823,10 @@ extern C { EAPI void evas_object_event_callback_add
Re: [E-devel] [RFC] Async events API
On Fri, Apr 25, 2008 at 10:15 AM, Cedric BAIL [EMAIL PROTECTED] wrote: On Thu, Apr 24, 2008 at 11:15 PM, Gustavo Sverzut Barbieri [EMAIL PROTECTED] wrote: On Thu, Apr 24, 2008 at 1:57 PM, Cedric BAIL [EMAIL PROTECTED] wrote: On Wed, Apr 23, 2008 at 4:45 PM, The Rasterman Carsten Haitzler [EMAIL PROTECTED] wrote: On Tue, 15 Apr 2008 13:15:13 -0300 Gustavo Sverzut Barbieri [EMAIL PROTECTED] babbled: For simplicity, I would just process one message per callback from ecore_fd_main... but we can also use a poll/select inside this function and do what you want, without the need to fcntl to NONBLOCKING. just read a buffer of messages - if we don't get a complete message, store the partial one and keep it until the next call to process events then complete the message fetch. Ok. Here is an updated patch. The fd is still nonblocking and on partial read, it should be able to just restart cleany at a later time. You will now pass a function pointer so that you can call whatever kind of code you want (I doubt we really need something else than evas_object_event_callback). I also make it optional. Some more comments ? configure: s/build_async/build_async_events/g; make it default on (use auto to check for pthread, yes would fail without pthread and no is... well no). +static pthread_mutex_t _mutex; i think it's safer to just initialize the mutex here using (and remove the destroy call): static pthread_mutex_t _mutex = PTHREAD_MUTEX_INITIALIZER; evas_async_events_process(void): + if (_fd_read != -1) + do reverse this logic: if (_d_read == -1) return 0; this will avoid the unnecessary nesting and would remove the bug that it have today: check might be uninitialized in this function. Actually write the whole function like (more beautiful imho): if (_fd_read == -1) return -1; count = 0; while ((check = read(_fd_read, ((char *)current) + size, sizeof(current) - size)) 0) { size += check; if (size sizeof(current)) continue; if (current.func) current.func(current.target, current.type, current.event_info); size = 0; count++; } if (check 0) { ...; _fd_read = -1; } return count; For evas_async_events_put, also reverse the logic, I'd write: if (!func) return 0; if (_fd_write == -1) return 0; ... pthread_mutex_lock(_mutex); do { ... } while ( ... ) pthread_mutex_unlock(_mutex); this would not do unneeded locks and avoid nested code. Also, note that the function return Evas_Bool but in the version without BUILD_ASYNC_EVENTS you return -1. Aside these minor, mostly cosmetic, things, it's ready for inclusion :-) Thanks a lot for taking the time to do the review. Here is a final patch, and if nobody oppose, I will commit it during next week. excellent, commit :-) /me just waiting for the threaded image loading... ;-) -- Gustavo Sverzut Barbieri http://profusion.mobi Embedded Systems -- MSN: [EMAIL PROTECTED] Skype: gsbarbieri Mobile: +55 (81) 9927 0010 - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ___ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
Re: [E-devel] [RFC] Async events API
On Thu, Apr 24, 2008 at 1:57 PM, Cedric BAIL [EMAIL PROTECTED] wrote: On Wed, Apr 23, 2008 at 4:45 PM, The Rasterman Carsten Haitzler [EMAIL PROTECTED] wrote: On Tue, 15 Apr 2008 13:15:13 -0300 Gustavo Sverzut Barbieri [EMAIL PROTECTED] babbled: For simplicity, I would just process one message per callback from ecore_fd_main... but we can also use a poll/select inside this function and do what you want, without the need to fcntl to NONBLOCKING. just read a buffer of messages - if we don't get a complete message, store the partial one and keep it until the next call to process events then complete the message fetch. Ok. Here is an updated patch. The fd is still nonblocking and on partial read, it should be able to just restart cleany at a later time. You will now pass a function pointer so that you can call whatever kind of code you want (I doubt we really need something else than evas_object_event_callback). I also make it optional. Some more comments ? configure: s/build_async/build_async_events/g; make it default on (use auto to check for pthread, yes would fail without pthread and no is... well no). +static pthread_mutex_t _mutex; i think it's safer to just initialize the mutex here using (and remove the destroy call): static pthread_mutex_t _mutex = PTHREAD_MUTEX_INITIALIZER; evas_async_events_process(void): + if (_fd_read != -1) + do reverse this logic: if (_d_read == -1) return 0; this will avoid the unnecessary nesting and would remove the bug that it have today: check might be uninitialized in this function. Actually write the whole function like (more beautiful imho): if (_fd_read == -1) return -1; count = 0; while ((check = read(_fd_read, ((char *)current) + size, sizeof(current) - size)) 0) { size += check; if (size sizeof(current)) continue; if (current.func) current.func(current.target, current.type, current.event_info); size = 0; count++; } if (check 0) { ...; _fd_read = -1; } return count; For evas_async_events_put, also reverse the logic, I'd write: if (!func) return 0; if (_fd_write == -1) return 0; ... pthread_mutex_lock(_mutex); do { ... } while ( ... ) pthread_mutex_unlock(_mutex); this would not do unneeded locks and avoid nested code. Also, note that the function return Evas_Bool but in the version without BUILD_ASYNC_EVENTS you return -1. Aside these minor, mostly cosmetic, things, it's ready for inclusion :-) -- Gustavo Sverzut Barbieri http://profusion.mobi Embedded Systems -- MSN: [EMAIL PROTECTED] Skype: gsbarbieri Mobile: +55 (81) 9927 0010 - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ___ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
Re: [E-devel] [RFC] Async events API
On Wed, 9 Apr 2008 15:55:57 +0200 Cedric BAIL [EMAIL PROTECTED] babbled: I arleady though about some other possibilities : - make it optional in configure - only start it when required, but that's much more complex - only initialise it from required sub system (like later the cache subsystem), but this will render useless the call to evas_async_events_process in some case. i think that non-blocking for reads are something we need. calling the process call needs to do nothing if there is nothing to read (and just return). it should be optional - sure, so things that would be done async are done inline without a thread if disabled (things just block instead). -- - Codito, ergo sum - I code, therefore I am -- The Rasterman (Carsten Haitzler)[EMAIL PROTECTED] - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ___ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
Re: [E-devel] [RFC] Async events API
On Tue, 15 Apr 2008 13:15:13 -0300 Gustavo Sverzut Barbieri [EMAIL PROTECTED] babbled: For simplicity, I would just process one message per callback from ecore_fd_main... but we can also use a poll/select inside this function and do what you want, without the need to fcntl to NONBLOCKING. just read a buffer of messages - if we don't get a complete message, store the partial one and keep it until the next call to process events then complete the message fetch. -- Gustavo Sverzut Barbieri http://profusion.mobi Embedded Systems -- MSN: [EMAIL PROTECTED] Skype: gsbarbieri Mobile: +55 (81) 9927 0010 -- - Codito, ergo sum - I code, therefore I am -- The Rasterman (Carsten Haitzler)[EMAIL PROTECTED] - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ___ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
Re: [E-devel] [RFC] Async events API
On Sun, 13 Apr 2008 13:35:58 -0300 Gustavo Sverzut Barbieri [EMAIL PROTECTED] babbled: On Sat, Apr 12, 2008 at 2:37 AM, The Rasterman Carsten Haitzler [EMAIL PROTECTED] wrote: no need to make it blocking :) look at ecore_con - hell look at emotion. it's Well, I looked at emotion and damn, it's wrong! :-P Ok, it will work for most of the cases where you are within the buffer size, but then it would also not block if you're in the same situation and have not set the non-blocking flag. The problem is at the border condition: full buffer: - blocking would block the call and make the sender process (in this case, the thread) to block a while... IMO this is not so bad, as the main process is not being capable of reading it at all. - non-blocking would write partial and stay looping to write the remaining bytes... so blocking the same way, or it would do a partial write if you don't loop. Also, on the read side, depending on the implementation it would either loop (so blocking) waiting for data or do partial reads (as it might do with Emotion), these partial reads are the damn bug-causing I'm talking about. read() and write() does not do all the work for us because these are really low level functions enabling us to do such a looping solution (that blocks in any way) or queueing the data elsewhere (todo-buffers) and doing something else if the read/write fails... but doing these todo-buffers is really a PITA, I never see nobody using them, maybe these are used in some high-performance servers that can keep todo-buffers for each socket and try to dispatch other sockets if some are blocked. Just read emotion code and consider: - Thread 1: write 3xmessage-size bytes (consider message-size being 2 bytes: [AA|BB|CC]) where? it writes void * sized messages. the vo_out plugin writes only 1 void * msg, the slave writes 2 * void *, the event thread writes 2 * void *. no 3x there? - Thread 2: read message-size bytes [AA], then read 1 byte [B]... it's not of message-size bytes, then discard (!!!) and try to read message-size bytes again, which you can do because you still have 3 bytes there, so you end with [BC], then you process this wrong pointer and then read message-size again, being unable because just [C] is there. hmm in practice this won't happen. thread 1 has written a full message into the fd - it's all available, or should be. :) as kernel buffers are 64k or 148k - but are always a page size multiple in reality, you will always have a whole number of messages in the buffer :) I don't even want to read ecore_con! If it resemble emotion then it's wrong and it might incur in security risk for all code using it... The risky is not that important since you don't handle pointers to a process to another, just between threads, and nobody would replace a thread... :-P ecore_con knows nothing of message sizes. it just reads as many bytes as it can and puts them into a buffer together then hands it off to the rest of the app as an event. What also makes me wonder: why not make this kind of thing available in ecore? Just handle read/write file descriptors to a function to have these configured (maybe also set the buffer size based on message size) and provide ecore_read() and ecore_write() with these loops all handled? This is way better than spreading these weird loops all over the code, many can be just wrong and will spread bugs :-/ that is what the talk of ecore_pipe was about. it would set up these pipes, fd handlers etc. and handle putting messages on the pipe queue and gettting them off and presenting them as events - much like ecore_con. all done with non-blocking. for emotion i used void *'s to write that point to he message. it works by handing the pointer to a msg struct from the slave thread to the main one - it's quite handy that way. it's also generic as i can pass anything along from a slave to a master thread - just point to it :) the difference between a byte and a void * won't really be anything worth talking about as u are unlikely to pass 1000's of them at a time - even if u do, kernel buffers are 128k by default - that's 32k messages on 32bit and 16k on 16bit that u can buffer before draining... that's a LOOOT. I guess you mean 16K on 64bit... :-) yeah! that's what i meant :) Ok, your example fits into the buffer size exactly, but we shouldn't trust this to be always true. Also, as I tried to show, the blocking code would just do the same thing and make the error-handling easier. i know it does :) that's why i did it :) but yes - block on WRITE would be good. blocking read is what i was thinking - u don't want to block :) -- Gustavo Sverzut Barbieri http://profusion.mobi Embedded Systems -- MSN: [EMAIL PROTECTED] Skype: gsbarbieri Mobile: +55 (81) 9927 0010 -- - Codito, ergo sum - I code, therefore I am -- The Rasterman
Re: [E-devel] [RFC] Async events API
On Tue, Apr 15, 2008 at 1:28 AM, The Rasterman Carsten Haitzler [EMAIL PROTECTED] wrote: On Sun, 13 Apr 2008 13:35:58 -0300 Gustavo Sverzut Barbieri [EMAIL PROTECTED] babbled: On Sat, Apr 12, 2008 at 2:37 AM, The Rasterman Carsten Haitzler [EMAIL PROTECTED] wrote: no need to make it blocking :) look at ecore_con - hell look at emotion. it's Well, I looked at emotion and damn, it's wrong! :-P Ok, it will work for most of the cases where you are within the buffer size, but then it would also not block if you're in the same situation and have not set the non-blocking flag. The problem is at the border condition: full buffer: - blocking would block the call and make the sender process (in this case, the thread) to block a while... IMO this is not so bad, as the main process is not being capable of reading it at all. - non-blocking would write partial and stay looping to write the remaining bytes... so blocking the same way, or it would do a partial write if you don't loop. Also, on the read side, depending on the implementation it would either loop (so blocking) waiting for data or do partial reads (as it might do with Emotion), these partial reads are the damn bug-causing I'm talking about. read() and write() does not do all the work for us because these are really low level functions enabling us to do such a looping solution (that blocks in any way) or queueing the data elsewhere (todo-buffers) and doing something else if the read/write fails... but doing these todo-buffers is really a PITA, I never see nobody using them, maybe these are used in some high-performance servers that can keep todo-buffers for each socket and try to dispatch other sockets if some are blocked. Just read emotion code and consider: - Thread 1: write 3xmessage-size bytes (consider message-size being 2 bytes: [AA|BB|CC]) where? it writes void * sized messages. the vo_out plugin writes only 1 void * msg, the slave writes 2 * void *, the event thread writes 2 * void *. no 3x there? it was just to make the example simpler, the simplest case. - Thread 2: read message-size bytes [AA], then read 1 byte [B]... it's not of message-size bytes, then discard (!!!) and try to read message-size bytes again, which you can do because you still have 3 bytes there, so you end with [BC], then you process this wrong pointer and then read message-size again, being unable because just [C] is there. hmm in practice this won't happen. thread 1 has written a full message into the fd - it's all available, or should be. :) as kernel buffers are 64k or 148k - but are always a page size multiple in reality, you will always have a whole number of messages in the buffer :) Ok, ok, but taking care of this will not increase complexity, but would make code more correct. I don't even want to read ecore_con! If it resemble emotion then it's wrong and it might incur in security risk for all code using it... The risky is not that important since you don't handle pointers to a process to another, just between threads, and nobody would replace a thread... :-P ecore_con knows nothing of message sizes. it just reads as many bytes as it can and puts them into a buffer together then hands it off to the rest of the app as an event. ic, then good... this is the case where it all works well: keep a buffer with partial reads, thatÅ› what we don't have in this thread-sync case. What also makes me wonder: why not make this kind of thing available in ecore? Just handle read/write file descriptors to a function to have these configured (maybe also set the buffer size based on message size) and provide ecore_read() and ecore_write() with these loops all handled? This is way better than spreading these weird loops all over the code, many can be just wrong and will spread bugs :-/ that is what the talk of ecore_pipe was about. it would set up these pipes, fd handlers etc. and handle putting messages on the pipe queue and gettting them off and presenting them as events - much like ecore_con. fine, so we agree :-) Ok, your example fits into the buffer size exactly, but we shouldn't trust this to be always true. Also, as I tried to show, the blocking code would just do the same thing and make the error-handling easier. i know it does :) that's why i did it :) but yes - block on WRITE would be good. blocking read is what i was thinking - u don't want to block :) in our case it won't matter! According to you, we WILL get the required data (multiple of page size, enough buffer size). But if we don't, then we do a busy-waiting looping on read(2) call to get the data... that's BAD. So we also block on READ, or we don't have any problem (from what you consider) or we block and don't suck 100% CPU (from what I said). For simplicity, I would just process one message
Re: [E-devel] [RFC] Async events API
On Tue, 8 Apr 2008 14:02:50 -0300 Gustavo Sverzut Barbieri [EMAIL PROTECTED] babbled: On Tue, Apr 8, 2008 at 11:05 AM, Cedric BAIL [EMAIL PROTECTED] wrote: Hi, This patch doesn't break anything at this time :-) It's a standalone feature that just add the possibility to evas to receive events from another thread. It introduce 3 new API. I'm not sure the fd must be set to non-block, this might cause more trouble than good. I'd make it blocker for now and handle things properly using select(), or believe that when the file descriptor is ready to read (from ecore_fd_main...) we can read everything (ie: no partial writes, no threads will die with incomplete writes). non-block is fine. that's what select() is for :) read the fd while there are things to read - then stop and let select tell u more is there to read. this fd is to let evas wake up the calling program for an event that happened asynchronously - so that's all we need. it really only needs to write 1 word or byte down the pipe as a signal. personally i'd write a void * (pointer) that is a pointer to a message (struct) of some sort. a standard one. it is the reader's job to take the struct, pass it on to some handler and free it when done. posting events is just a matter of waking the process up. you want to do this for example when an image has finished decoding (maybe if u want progressive load u do it multiple times during load). this is very useful. :) * Retrieve the fd that need to be monitored by ecore : EAPI int evas_async_events_fd_get(); missing (void) yeah. :) * Process all pending event in the pipe (This function could be called without any event in the pipe) : EAPI int evas_async_events_process(); missing (void), also true for init, shutdown... also, do ... while (check == sizeof(current)) logic is wrong, you should check += read(..., todo) and also change where to load the rest of it. Actually I would rewrite it as: int todo = sizeof(current); char *p = (char *)current; do { int n; n = read(_fd_read, p, todo); if (n == -1) { if (errno == EAGAIN || errno == EINTR) continue; else HANDLE_ERROR(); } todo -= n; p += n; } while (todo 0) * Push an event inside the pipe from another thread : EAPI Evas_Bool evas_async_events_put(Evas_Object *obj, Evas_Callback_Type type, void *event_info); Write logic is also wrong. new + offset given that new is of type Evas_Event_Async is not what you want. This pointer arithmetic is like vector indexing, so it would be like: new[offset] You need to cast it to some type that is the size of a byte, like char: ((char *)new) + offset is fine. But I would rewrite it like I did for read. I currently don't have any code using it, but I plan to use this for the coming background loading of image. So please review it. done :-) I'm not sure about evas_init() doing this. Nothing against it, but maybe some purist wouldn't like to have pthread mutexes created. i would make this optional. if u are never going to use async stuff - it should just do nothing. the fd should just never wake up, or what is done in a thread can be just done in-line and the wakeup can be emulated. -- Gustavo Sverzut Barbieri http://profusion.mobi Embedded Systems -- MSN: [EMAIL PROTECTED] Skype: gsbarbieri Mobile: +55 (81) 9927 0010 - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Register now and save $200. Hurry, offer ends at 11:59 p.m., Monday, April 7! Use priority code J8TLD2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ___ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel -- - Codito, ergo sum - I code, therefore I am -- The Rasterman (Carsten Haitzler)[EMAIL PROTECTED] - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ___ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
Re: [E-devel] [RFC] Async events API
On Fri, Apr 11, 2008 at 9:43 PM, The Rasterman Carsten Haitzler [EMAIL PROTECTED] wrote: On Tue, 8 Apr 2008 14:02:50 -0300 Gustavo Sverzut Barbieri [EMAIL PROTECTED] babbled: On Tue, Apr 8, 2008 at 11:05 AM, Cedric BAIL [EMAIL PROTECTED] wrote: Hi, This patch doesn't break anything at this time :-) It's a standalone feature that just add the possibility to evas to receive events from another thread. It introduce 3 new API. I'm not sure the fd must be set to non-block, this might cause more trouble than good. I'd make it blocker for now and handle things properly using select(), or believe that when the file descriptor is ready to read (from ecore_fd_main...) we can read everything (ie: no partial writes, no threads will die with incomplete writes). non-block is fine. that's what select() is for :) read the fd while there are things to read - then stop and let select tell u more is there to read. this fd is to let evas wake up the calling program for an event that happened asynchronously - so that's all we need. it really only needs to write 1 word or byte down the pipe as a signal. personally i'd write a void * (pointer) that is a pointer to a message (struct) of some sort. a standard one. it is the reader's job to take the struct, pass it on to some handler and free it when done. posting events is just a matter of waking the process up. you want to do this for example when an image has finished decoding (maybe if u want progressive load u do it multiple times during load). this is very useful. :) I also agree with just using pipe to wakeup, but I'd go with a byte. Problem is that if we read more data from a non-blocking fd, then we might end with partial reads, then we must keep this somewhere for future readings... THAT's a problem and that's painful to get right with non-blocking, if we don't take care, bugs may show and they would be hard to track. Bytes are atomic there, just read one byte, pop from the queue and process, then read another, pop, process... I'm not sure about evas_init() doing this. Nothing against it, but maybe some purist wouldn't like to have pthread mutexes created. i would make this optional. if u are never going to use async stuff - it should just do nothing. the fd should just never wake up, or what is done in a thread can be just done in-line and the wakeup can be emulated. make what optional, and where? Make it whole async stuff compile-time optional? -- Gustavo Sverzut Barbieri http://profusion.mobi Embedded Systems -- MSN: [EMAIL PROTECTED] Skype: gsbarbieri Mobile: +55 (81) 9927 0010 - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ___ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
Re: [E-devel] [RFC] Async events API
On Fri, 11 Apr 2008 23:28:29 -0300 Gustavo Sverzut Barbieri [EMAIL PROTECTED] babbled: On Fri, Apr 11, 2008 at 9:43 PM, The Rasterman Carsten Haitzler [EMAIL PROTECTED] wrote: On Tue, 8 Apr 2008 14:02:50 -0300 Gustavo Sverzut Barbieri [EMAIL PROTECTED] babbled: On Tue, Apr 8, 2008 at 11:05 AM, Cedric BAIL [EMAIL PROTECTED] wrote: Hi, This patch doesn't break anything at this time :-) It's a standalone feature that just add the possibility to evas to receive events from another thread. It introduce 3 new API. I'm not sure the fd must be set to non-block, this might cause more trouble than good. I'd make it blocker for now and handle things properly using select(), or believe that when the file descriptor is ready to read (from ecore_fd_main...) we can read everything (ie: no partial writes, no threads will die with incomplete writes). non-block is fine. that's what select() is for :) read the fd while there are things to read - then stop and let select tell u more is there to read. this fd is to let evas wake up the calling program for an event that happened asynchronously - so that's all we need. it really only needs to write 1 word or byte down the pipe as a signal. personally i'd write a void * (pointer) that is a pointer to a message (struct) of some sort. a standard one. it is the reader's job to take the struct, pass it on to some handler and free it when done. posting events is just a matter of waking the process up. you want to do this for example when an image has finished decoding (maybe if u want progressive load u do it multiple times during load). this is very useful. :) I also agree with just using pipe to wakeup, but I'd go with a byte. Problem is that if we read more data from a non-blocking fd, then we might end with partial reads, then we must keep this somewhere for future readings... THAT's a problem and that's painful to get right with non-blocking, if we don't take care, bugs may show and they would be hard to track. Bytes are atomic there, just read one byte, pop from the queue and process, then read another, pop, process... no need to make it blocking :) look at ecore_con - hell look at emotion. it's all done with non-blocking. for emotion i used void *'s to write that point to he message. it works by handing the pointer to a msg struct from the slave thread to the main one - it's quite handy that way. it's also generic as i can pass anything along from a slave to a master thread - just point to it :) the difference between a byte and a void * won't really be anything worth talking about as u are unlikely to pass 1000's of them at a time - even if u do, kernel buffers are 128k by default - that's 32k messages on 32bit and 16k on 16bit that u can buffer before draining... that's a LOOOT. I'm not sure about evas_init() doing this. Nothing against it, but maybe some purist wouldn't like to have pthread mutexes created. i would make this optional. if u are never going to use async stuff - it should just do nothing. the fd should just never wake up, or what is done in a thread can be just done in-line and the wakeup can be emulated. make what optional, and where? Make it whole async stuff compile-time optional? no - the async fd is there, but the operation is not done in a thread. the load is done syncronously at load time if async stuff is disabled. u just need to have infrastructure where u can set a callback to be executed async - it will be (and then message back the event fd with a message when it's done) *IF* async evas stuff is enabled (enable by default), *IF it's not enabled, the callback will be called at the time you start the async operation (i.e at the time u begin the load). t5his means the load call, so to speak, will block and be slow until the async callback finishes, if async is disabled, and if enabled, it won't block. in both situations your async fd will wake up with an event you process later :) -- - Codito, ergo sum - I code, therefore I am -- The Rasterman (Carsten Haitzler)[EMAIL PROTECTED] - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Don't miss this year's exciting event. There's still time to save $100. Use priority code J8TL2D2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ___ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
Re: [E-devel] [RFC] Async events API
On Tue, Apr 8, 2008 at 7:02 PM, Gustavo Sverzut Barbieri [EMAIL PROTECTED] wrote: On Tue, Apr 8, 2008 at 11:05 AM, Cedric BAIL [EMAIL PROTECTED] wrote: Hi, This patch doesn't break anything at this time :-) It's a standalone feature that just add the possibility to evas to receive events from another thread. It introduce 3 new API. I'm not sure the fd must be set to non-block, this might cause more trouble than good. I'd make it blocker for now and handle things properly using select(), or believe that when the file descriptor is ready to read (from ecore_fd_main...) we can read everything (ie: no partial writes, no threads will die with incomplete writes). I would like to have more opinion on that matter. I still like the idea of non blocking as it give the possibility to call evas_async_events_process from pure evas program. * Retrieve the fd that need to be monitored by ecore : EAPI int evas_async_events_fd_get(); missing (void) * Process all pending event in the pipe (This function could be called without any event in the pipe) : EAPI int evas_async_events_process(); missing (void), also true for init, shutdown... Oops ! also, do ... while (check == sizeof(current)) logic is wrong, you should check += read(..., todo) and also change where to load the rest of it. Actually I would rewrite it as: int todo = sizeof(current); char *p = (char *)current; do { int n; n = read(_fd_read, p, todo); if (n == -1) { if (errno == EAGAIN || errno == EINTR) continue; else HANDLE_ERROR(); } todo -= n; p += n; } while (todo 0) I guess it will not work with non blocking fd. I did correct it by taking non blocking fd into account. * Push an event inside the pipe from another thread : EAPI Evas_Bool evas_async_events_put(Evas_Object *obj, Evas_Callback_Type type, void *event_info); Write logic is also wrong. new + offset given that new is of type Evas_Event_Async is not what you want. This pointer arithmetic is like vector indexing, so it would be like: new[offset] You need to cast it to some type that is the size of a byte, like char: ((char *)new) + offset is fine. But I would rewrite it like I did for read. Oops, corrected. I currently don't have any code using it, but I plan to use this for the coming background loading of image. So please review it. done :-) Thanks a lot for this good and fast review ! I'm not sure about evas_init() doing this. Nothing against it, but maybe some purist wouldn't like to have pthread mutexes created. I arleady though about some other possibilities : - make it optional in configure - only start it when required, but that's much more complex - only initialise it from required sub system (like later the cache subsystem), but this will render useless the call to evas_async_events_process in some case. So more opinion are welcome :-) And for keeping you interested to the subject, here is the updated patch. -- Cedric BAIL diff --git a/src/lib/Evas.h b/src/lib/Evas.h index e78e9a9..e1e05a1 100644 --- a/src/lib/Evas.h +++ b/src/lib/Evas.h @@ -815,6 +815,10 @@ extern C { EAPI void evas_object_event_callback_add(Evas_Object *obj, Evas_Callback_Type type, void (*func) (void *data, Evas *e, Evas_Object *obj, void *event_info), const void *data); EAPI void *evas_object_event_callback_del(Evas_Object *obj, Evas_Callback_Type type, void (*func) (void *data, Evas *e, Evas_Object *obj, void *event_info)); + EAPI int evas_async_events_fd_get (void); + EAPI int evas_async_events_process (void); + EAPI Evas_Bool evas_async_events_put (Evas_Object *obj, Evas_Callback_Type type, void *event_info); + EAPI void evas_object_intercept_show_callback_add(Evas_Object *obj, void (*func) (void *data, Evas_Object *obj), const void *data); EAPI void *evas_object_intercept_show_callback_del(Evas_Object *obj, void (*func) (void *data, Evas_Object *obj)); EAPI void evas_object_intercept_hide_callback_add(Evas_Object *obj, void (*func) (void *data, Evas_Object *obj), const void *data); diff --git a/src/lib/canvas/Makefile.am b/src/lib/canvas/Makefile.am index 9dda0e1..db66ba6 100644 --- a/src/lib/canvas/Makefile.am +++ b/src/lib/canvas/Makefile.am @@ -37,6 +37,7 @@ evas_font_dir.c \ evas_rectangle.c \ evas_render.c \ evas_smart.c \ -evas_stack.c +evas_stack.c \ +evas_async_events.c libevas_canvas_la_DEPENDENCIES = $(top_builddir)/config.h diff --git a/src/lib/canvas/evas_async_events.c b/src/lib/canvas/evas_async_events.c new file mode 100644 index 000..117ef0f --- /dev/null +++ b/src/lib/canvas/evas_async_events.c @@ -0,0 +1,128 @@ +#include evas_common.h +#include evas_private.h + +#include unistd.h +#include fcntl.h
[E-devel] [RFC] Async events API
Hi, This patch doesn't break anything at this time :-) It's a standalone feature that just add the possibility to evas to receive events from another thread. It introduce 3 new API. * Retrieve the fd that need to be monitored by ecore : EAPI int evas_async_events_fd_get(); * Process all pending event in the pipe (This function could be called without any event in the pipe) : EAPI int evas_async_events_process(); * Push an event inside the pipe from another thread : EAPI Evas_Bool evas_async_events_put(Evas_Object *obj, Evas_Callback_Type type, void *event_info); I currently don't have any code using it, but I plan to use this for the coming background loading of image. So please review it. -- Cedric BAIL diff --git a/src/lib/Evas.h b/src/lib/Evas.h index e78e9a9..06f8a83 100644 --- a/src/lib/Evas.h +++ b/src/lib/Evas.h @@ -815,6 +815,10 @@ extern C { EAPI void evas_object_event_callback_add(Evas_Object *obj, Evas_Callback_Type type, void (*func) (void *data, Evas *e, Evas_Object *obj, void *event_info), const void *data); EAPI void *evas_object_event_callback_del(Evas_Object *obj, Evas_Callback_Type type, void (*func) (void *data, Evas *e, Evas_Object *obj, void *event_info)); + EAPI int evas_async_events_fd_get (); + EAPI int evas_async_events_process (); + EAPI Evas_Bool evas_async_events_put (Evas_Object *obj, Evas_Callback_Type type, void *event_info); + EAPI void evas_object_intercept_show_callback_add(Evas_Object *obj, void (*func) (void *data, Evas_Object *obj), const void *data); EAPI void *evas_object_intercept_show_callback_del(Evas_Object *obj, void (*func) (void *data, Evas_Object *obj)); EAPI void evas_object_intercept_hide_callback_add(Evas_Object *obj, void (*func) (void *data, Evas_Object *obj), const void *data); diff --git a/src/lib/canvas/Makefile.am b/src/lib/canvas/Makefile.am index 9dda0e1..db66ba6 100644 --- a/src/lib/canvas/Makefile.am +++ b/src/lib/canvas/Makefile.am @@ -37,6 +37,7 @@ evas_font_dir.c \ evas_rectangle.c \ evas_render.c \ evas_smart.c \ -evas_stack.c +evas_stack.c \ +evas_async_events.c libevas_canvas_la_DEPENDENCIES = $(top_builddir)/config.h diff --git a/src/lib/canvas/evas_async_events.c b/src/lib/canvas/evas_async_events.c new file mode 100644 index 000..fb21c02 --- /dev/null +++ b/src/lib/canvas/evas_async_events.c @@ -0,0 +1,121 @@ +#include evas_common.h +#include evas_private.h + +#include unistd.h +#include fcntl.h +#include pthread.h +#include errno.h + +static int _fd_write = -1; +static int _fd_read = -1; + +static int _init_evas_event = 0; +static pthread_mutex_t _mutex; + +typedef struct _Evas_Event_Async Evas_Event_Async; +struct _Evas_Event_Async +{ + Evas_Object *obj; + Evas_Callback_Type type; + void *event_info; +}; + +int +evas_async_events_init() +{ + if (_init_evas_event++ == 0) + { + int filedes[2]; + + if (pipe(filedes) == -1) + { + _init_evas_event = 0; + return 0; + } + + _fd_read = filedes[0]; + _fd_write = filedes[1]; + + fcntl(_fd_read, F_SETFL, O_NONBLOCK); + + pthread_mutex_init(_mutex, NULL); + } + + return _init_evas_event; +} + +int +evas_async_events_shutdown() +{ + if (--_init_evas_event == 0) + { + close(_fd_read); + close(_fd_write); + _fd_read = -1; + _fd_write = -1; + + pthread_mutex_destroy(_mutex); + } + + return _init_evas_event; +} + +EAPI int +evas_async_events_fd_get() +{ + return _fd_read; +} + +EAPI int +evas_async_events_process() +{ + Evas_Event_Async current; + int check; + int count = 0; + + do + { + check = read(_fd_read, current, sizeof(current)); + + if (check == sizeof(current)) + { + evas_object_event_callback_call(current.obj, current.type, current.event_info); + ++count; + } + } + while (check == sizeof(current)); + + return count; +} + +EAPI Evas_Bool +evas_async_events_put(Evas_Object *obj, Evas_Callback_Type type, void *event_info) +{ + Evas_Event_Async new; + Evas_Bool result; + + result = 0; + + new.obj = obj; + new.type = type; + new.event_info = event_info; + + pthread_mutex_lock(_mutex); + if (_fd_write != -1) + { + ssize_t check; + int offset = 0; + + do { + check = write(_fd_write, new + offset, sizeof(new) - offset); + offset += check; + } while (offset != sizeof(new) (errno == EINTR || errno == EAGAIN)); + + if (offset == sizeof(new)) + result = 1; + } + pthread_mutex_unlock(_mutex); + + return result; +} + diff --git a/src/lib/canvas/evas_main.c b/src/lib/canvas/evas_main.c index 7313a33..ca1b8dc 100644 --- a/src/lib/canvas/evas_main.c +++ b/src/lib/canvas/evas_main.c @@ -7,8 +7,12 @@ static int initcount = 0; EAPI int evas_init(void) { + if (evas_async_events_init() == 0) + return 0; + if (initcount == 0) evas_module_init(); + return ++initcount; } @@ -22,6 +26,7 @@
Re: [E-devel] [RFC] Async events API
On Tue, Apr 8, 2008 at 11:05 AM, Cedric BAIL [EMAIL PROTECTED] wrote: Hi, This patch doesn't break anything at this time :-) It's a standalone feature that just add the possibility to evas to receive events from another thread. It introduce 3 new API. I'm not sure the fd must be set to non-block, this might cause more trouble than good. I'd make it blocker for now and handle things properly using select(), or believe that when the file descriptor is ready to read (from ecore_fd_main...) we can read everything (ie: no partial writes, no threads will die with incomplete writes). * Retrieve the fd that need to be monitored by ecore : EAPI int evas_async_events_fd_get(); missing (void) * Process all pending event in the pipe (This function could be called without any event in the pipe) : EAPI int evas_async_events_process(); missing (void), also true for init, shutdown... also, do ... while (check == sizeof(current)) logic is wrong, you should check += read(..., todo) and also change where to load the rest of it. Actually I would rewrite it as: int todo = sizeof(current); char *p = (char *)current; do { int n; n = read(_fd_read, p, todo); if (n == -1) { if (errno == EAGAIN || errno == EINTR) continue; else HANDLE_ERROR(); } todo -= n; p += n; } while (todo 0) * Push an event inside the pipe from another thread : EAPI Evas_Bool evas_async_events_put(Evas_Object *obj, Evas_Callback_Type type, void *event_info); Write logic is also wrong. new + offset given that new is of type Evas_Event_Async is not what you want. This pointer arithmetic is like vector indexing, so it would be like: new[offset] You need to cast it to some type that is the size of a byte, like char: ((char *)new) + offset is fine. But I would rewrite it like I did for read. I currently don't have any code using it, but I plan to use this for the coming background loading of image. So please review it. done :-) I'm not sure about evas_init() doing this. Nothing against it, but maybe some purist wouldn't like to have pthread mutexes created. -- Gustavo Sverzut Barbieri http://profusion.mobi Embedded Systems -- MSN: [EMAIL PROTECTED] Skype: gsbarbieri Mobile: +55 (81) 9927 0010 - This SF.net email is sponsored by the 2008 JavaOne(SM) Conference Register now and save $200. Hurry, offer ends at 11:59 p.m., Monday, April 7! Use priority code J8TLD2. http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone ___ enlightenment-devel mailing list enlightenment-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/enlightenment-devel