raster pushed a commit to branch master. http://git.enlightenment.org/core/efl.git/commit/?id=be29f3f4ecfda76de98963b5f2d04c0ccc672590
commit be29f3f4ecfda76de98963b5f2d04c0ccc672590 Author: Carsten Haitzler (Rasterman) <ras...@rasterman.com> Date: Thu Sep 8 15:00:22 2016 +0900 efl ui image - fix janky blocking async image preload thread handling fix the elm image threaded image preload to be far simpler and actually threadsafe without blocking the mainloop at all even on object deletion. this also ensures ar least the first 512M of any async precached file are loaded in so the preload doesnt stall on headers that are outside maybe the first 4k of the file. i saw this happening all over the place in the test i created. @optimize --- src/lib/elementary/efl_ui_image.c | 334 ++++++++++++------------------- src/lib/elementary/efl_ui_widget_image.h | 6 +- 2 files changed, 132 insertions(+), 208 deletions(-) diff --git a/src/lib/elementary/efl_ui_image.c b/src/lib/elementary/efl_ui_image.c index 0ddab23..b1ddcc5 100644 --- a/src/lib/elementary/efl_ui_image.c +++ b/src/lib/elementary/efl_ui_image.c @@ -44,12 +44,15 @@ static const Elm_Action key_actions[] = { {NULL, NULL} }; -struct _Async_Open_Data { +typedef struct _Async_Open_Data Async_Open_Data; + +struct _Async_Open_Data +{ + Eo *obj; Eina_Stringshare *file, *key; - Eina_File *f_set, *f_open; - void *map; - Eina_Bool cancel; - Eina_Bool failed; + Eina_File *f_set, *f_open; + void *map; + unsigned char sum; }; static void @@ -241,243 +244,186 @@ _async_open_data_free(Async_Open_Data *data) } static void -_efl_ui_image_async_open_do(void *data, Ecore_Thread *thread EINA_UNUSED) +_efl_ui_image_async_open_do(void *data, Ecore_Thread *thread) { - Efl_Ui_Image_Data * sd = data; + Async_Open_Data *todo = data; Eina_File *f; - Async_Open_Data *todo, *done; void *map = NULL; - unsigned int buf; + unsigned char *p, sum = 0; + size_t i, size; - eina_spinlock_take(&sd->async.lck); - todo = sd->async.todo; - done = sd->async.done; - sd->async.todo = NULL; - sd->async.done = NULL; - - if (done) _async_open_data_free(done); - if (!todo) - { - eina_spinlock_release(&sd->async.lck); - return; - } + if (ecore_thread_check(thread)) return; -begin: - if (todo->f_set) - f = todo->f_set; + if (todo->f_set) f = todo->f_set; else if (todo->file) { // blocking f = eina_file_open(todo->file, EINA_FALSE); - if (!f) - { - todo->failed = EINA_TRUE; - sd->async.done = todo; - eina_spinlock_release(&sd->async.lck); - return; - } + if (!f) return; } else { CRI("Async open has no input file!"); - eina_spinlock_release(&sd->async.lck); return; } - - if (ecore_thread_check(sd->async.th)) + if (ecore_thread_check(thread)) { if (!todo->f_set) eina_file_close(f); - _async_open_data_free(todo); - eina_spinlock_release(&sd->async.lck); return; } // Read just enough data for map to actually do something. - map = eina_file_map_all(f, EINA_FILE_SEQUENTIAL); - if (map && (eina_file_size_get(f) >= sizeof(buf))) - memcpy(&buf, map, sizeof(buf)); + p = map = eina_file_map_all(f, EINA_FILE_SEQUENTIAL); + size = eina_file_size_get(f); + // Read and ensure all pages are in memory for sure first just + // 1 byte per page will do. also keep a limit on how much we will + // blindly load in here to let's say 512M + if (size > (512 * 1024 * 1024)) size = 512 * 1024 * 1024; + for (i = 0; i < size; i += 4096) + { + if (ecore_thread_check(thread)) break; + sum += p[i]; + } - if (ecore_thread_check(sd->async.th)) + if (ecore_thread_check(thread)) { if (map) eina_file_map_free(f, map); if (!todo->f_set) eina_file_close(f); - _async_open_data_free(todo); - eina_spinlock_release(&sd->async.lck); return; } + todo->f_open = f; + todo->map = map; + todo->sum = sum; +} - done = todo; - todo = NULL; - done->cancel = EINA_FALSE; - done->failed = EINA_FALSE; - done->f_set = NULL; - done->f_open = f; - done->map = map; - - todo = sd->async.todo; +static void +_async_clear(Efl_Ui_Image_Data *sd) +{ + sd->async.th = NULL; sd->async.todo = NULL; - if (!todo) sd->async.done = done; + eina_stringshare_del(sd->async.file); + eina_stringshare_del(sd->async.key); + sd->async.file = NULL; + sd->async.key = NULL; +} - if (todo) +static void +_async_cancel(Efl_Ui_Image_Data *sd) +{ + if (sd->async.th) { - DBG("Async open got a new command before finishing"); - _async_open_data_free(done); - goto begin; + ecore_thread_cancel(sd->async.th); + ((Async_Open_Data *)(sd->async.todo))->obj = NULL; + _async_clear(sd); } - eina_spinlock_release(&sd->async.lck); } static void -_efl_ui_image_async_open_cancel(void *data, Ecore_Thread *thread EINA_UNUSED) +_efl_ui_image_async_open_cancel(void *data, Ecore_Thread *thread) { - Efl_Ui_Image_Data * sd = data; + Async_Open_Data *todo = data; DBG("Async open thread was canceled"); - sd->async.th = NULL; - sd->async_opening = EINA_FALSE; - sd->async_failed = EINA_TRUE; - // keep file, key for file_get() + if (todo->obj) + { + EFL_UI_IMAGE_DATA_GET(todo->obj, sd); + if (sd) + { + if (thread == sd->async.th) _async_clear(sd); + } + } + _async_open_data_free(todo); } static void -_efl_ui_image_async_open_done(void *data, Ecore_Thread *thread EINA_UNUSED) +_efl_ui_image_async_open_done(void *data, Ecore_Thread *thread) { - Efl_Ui_Image_Data * sd = data; + Async_Open_Data *todo = data; Eina_Stringshare *file, *key; - Async_Open_Data *done; Eina_Bool ok; Eina_File *f; void *map; - // async open thread can't be running now - // locking anyways to be sure (memory barrier), see CID 1343345 - eina_spinlock_take(&sd->async.lck); - - sd->async.th = NULL; - sd->async_failed = EINA_FALSE; - - if (sd->async.todo) - { - eina_spinlock_release(&sd->async.lck); - sd->async.th = ecore_thread_run(_efl_ui_image_async_open_do, - _efl_ui_image_async_open_done, - _efl_ui_image_async_open_cancel, sd); - - return; - } - - sd->async_opening = EINA_FALSE; - - done = sd->async.done; - if (!done) - { - // done should be NULL only after cancel... not here - ERR("Async open failed for an unknown reason."); - sd->async_failed = EINA_TRUE; - eina_spinlock_release(&sd->async.lck); - efl_event_callback_legacy_call(sd->self, EFL_FILE_EVENT_ASYNC_ERROR, NULL); - return; - } - - DBG("Async open succeeded"); - sd->async.done = NULL; - eina_spinlock_release(&sd->async.lck); - key = done->key; - map = done->map; - f = done->f_open; - ok = f && map; - if (done->file) - file = done->file; - else - file = f ? eina_file_filename_get(f) : NULL; - - if (ok) - { - if (sd->edje) - ok = edje_object_mmap_set(sd->img, f, key); - else - ok = _efl_ui_image_smart_internal_file_set(sd->self, sd, file, f, key); - } - - if (ok) + if (todo->obj) { - // TODO: Implement Efl.File async,opened event_info type - sd->async_failed = EINA_FALSE; - ELM_SAFE_FREE(sd->async.file, eina_stringshare_del); - ELM_SAFE_FREE(sd->async.key, eina_stringshare_del); - efl_event_callback_legacy_call(sd->self, EFL_FILE_EVENT_ASYNC_OPENED, NULL); - _efl_ui_image_internal_sizing_eval(sd->self, sd); - } - else - { - // TODO: Implement Efl.File async,error event_info type - sd->async_failed = EINA_TRUE; - // keep file,key around for file_get - efl_event_callback_legacy_call(sd->self, EFL_FILE_EVENT_ASYNC_ERROR, NULL); + EFL_UI_IMAGE_DATA_GET(todo->obj, sd); + if (sd) + { + if (thread == sd->async.th) + { + DBG("Async open succeeded"); + _async_clear(sd); + key = todo->key; + map = todo->map; + f = todo->f_open; + ok = f && map; + if (todo->file) file = todo->file; + else file = f ? eina_file_filename_get(f) : NULL; + + if (ok) + { + if (sd->edje) + ok = edje_object_mmap_set(sd->img, f, key); + else + ok = _efl_ui_image_smart_internal_file_set + (sd->self, sd, file, f, key); + } + if (ok) + { + // TODO: Implement Efl.File async,opened event_info type + efl_event_callback_legacy_call + (sd->self, EFL_FILE_EVENT_ASYNC_OPENED, NULL); + _efl_ui_image_internal_sizing_eval(sd->self, sd); + } + else + { + // keep file,key around for file_get + efl_event_callback_legacy_call + (sd->self, EFL_FILE_EVENT_ASYNC_ERROR, NULL); + } + } + } } - // close f, map and free strings - _async_open_data_free(done); + _async_open_data_free(todo); } static Eina_Bool -_efl_ui_image_async_file_set(Eo *obj EINA_UNUSED, Efl_Ui_Image_Data *sd, - const char *file, const Eina_File *f, const char *key) +_efl_ui_image_async_file_set(Eo *obj, Efl_Ui_Image_Data *sd, const char *file, + const Eina_File *f, const char *key) { Async_Open_Data *todo; - Eina_Bool was_running; - if (sd->async_opening && + if (sd->async.th && ((file == sd->async.file) || (file && sd->async.file && !strcmp(file, sd->async.file))) && ((key == sd->async.key) || (key && sd->async.key && !strcmp(key, sd->async.key)))) return EINA_TRUE; - sd->async_opening = EINA_TRUE; - eina_stringshare_replace(&sd->async.file, file); - eina_stringshare_replace(&sd->async.key, key); - - if (!sd->async.th) - { - was_running = EINA_FALSE; - sd->async.th = ecore_thread_run(_efl_ui_image_async_open_do, - _efl_ui_image_async_open_done, - _efl_ui_image_async_open_cancel, sd); - } - else was_running = EINA_TRUE; - - if (!sd->async.th) - { - DBG("Could not spawn an async thread!"); - sd->async_failed = EINA_TRUE; - return EINA_FALSE; - } - todo = calloc(1, sizeof(Async_Open_Data)); - if (!todo) - { - sd->async_failed = EINA_TRUE; - return EINA_FALSE; - } + if (!todo) return EINA_FALSE; + _async_cancel(sd); + + todo->obj = obj; todo->file = eina_stringshare_add(file); todo->key = eina_stringshare_add(key); todo->f_set = f ? eina_file_dup(f) : NULL; - if (!was_running) - sd->async.todo = todo; - else - { - Async_Open_Data *prev; - eina_spinlock_take(&sd->async.lck); - prev = sd->async.todo; - sd->async.todo = todo; - eina_spinlock_release(&sd->async.lck); - _async_open_data_free(prev); - } - sd->async_failed = EINA_FALSE; - return EINA_TRUE; + eina_stringshare_replace(&sd->async.file, file); + eina_stringshare_replace(&sd->async.key, key); + + sd->async.todo = todo; + sd->async.th = ecore_thread_run(_efl_ui_image_async_open_do, + _efl_ui_image_async_open_done, + _efl_ui_image_async_open_cancel, todo); + if (sd->async.th) return EINA_TRUE; + + _async_open_data_free(todo); + _async_clear(sd); + DBG("Could not spawn an async thread!"); + return EINA_FALSE; } static Eina_Bool @@ -504,6 +450,8 @@ _efl_ui_image_edje_file_set(Evas_Object *obj, evas_object_clip_set(sd->img, pclip); } + _async_cancel(sd); + sd->edje = EINA_TRUE; if (!sd->async_enable) { @@ -597,7 +545,6 @@ _efl_ui_image_efl_canvas_group_group_add(Eo *obj, Efl_Ui_Image_Data *priv) priv->scale_down = EINA_TRUE; priv->align_x = 0.5; priv->align_y = 0.5; - eina_spinlock_new(&priv->async.lck); elm_widget_can_focus_set(obj, EINA_FALSE); @@ -613,25 +560,7 @@ _efl_ui_image_efl_canvas_group_group_del(Eo *obj, Efl_Ui_Image_Data *sd) if (sd->remote) _elm_url_cancel(sd->remote); free(sd->remote_data); eina_stringshare_del(sd->key); - - if (sd->async.th) - { - if (!ecore_thread_cancel(sd->async.th) && - !ecore_thread_wait(sd->async.th, 1.0)) - { - ERR("Async open thread timed out during cancellation."); - // skipping all other data free to avoid crashes (this leaks) - efl_canvas_group_del(efl_super(obj, MY_CLASS)); - return; - } - sd->async.th = NULL; - } - - _async_open_data_free(sd->async.done); - _async_open_data_free(sd->async.todo); - eina_stringshare_del(sd->async.file); - eina_stringshare_del(sd->async.key); - + _async_cancel(sd); efl_canvas_group_del(efl_super(obj, MY_CLASS)); } @@ -891,6 +820,8 @@ _efl_ui_image_efl_file_mmap_set(Eo *obj, Efl_Ui_Image_Data *sd, if (sd->remote) _elm_url_cancel(sd->remote); sd->remote = NULL; + _async_cancel(sd); + if (!sd->async_enable) ret = _efl_ui_image_smart_internal_file_set(obj, sd, eina_file_filename_get(file), file, key); else @@ -1053,6 +984,8 @@ _efl_ui_image_efl_file_file_set(Eo *obj, Efl_Ui_Image_Data *sd, const char *file break; } + _async_cancel(sd); + if (!sd->async_enable) ret = _efl_ui_image_smart_internal_file_set(obj, sd, file, NULL, key); else @@ -1105,13 +1038,12 @@ _efl_ui_image_edje_object_size_min_calc(Eo *obj EINA_UNUSED, Efl_Ui_Image_Data * EOLIAN static void _efl_ui_image_efl_file_file_get(Eo *obj EINA_UNUSED, Efl_Ui_Image_Data *sd, const char **file, const char **key) { - if (sd->async_enable && (sd->async_opening || sd->async_failed)) + if (sd->async.th) { if (file) *file = sd->async.file; if (key) *key = sd->async.key; return; } - evas_object_image_file_get(sd->img, file, key); } @@ -1119,7 +1051,6 @@ static Eina_Bool _efl_ui_image_efl_file_async_wait(const Eo *obj EINA_UNUSED, Efl_Ui_Image_Data *pd) { Eina_Bool ok = EINA_TRUE; - if (!pd->async_enable) return ok; if (!pd->async.th) return ok; if (!ecore_thread_wait(pd->async.th, 1.0)) { @@ -1130,14 +1061,11 @@ _efl_ui_image_efl_file_async_wait(const Eo *obj EINA_UNUSED, Efl_Ui_Image_Data * } EOLIAN static void -_efl_ui_image_efl_file_async_set(Eo *obj, Efl_Ui_Image_Data *pd, Eina_Bool async) +_efl_ui_image_efl_file_async_set(Eo *obj EINA_UNUSED, Efl_Ui_Image_Data *pd, Eina_Bool async) { - if (pd->async_enable == async) - return; - + if (pd->async_enable == async) return; pd->async_enable = async; - if (!async) - _efl_ui_image_efl_file_async_wait(obj, pd); + if (!async) _async_cancel(pd); } EOLIAN static Eina_Bool diff --git a/src/lib/elementary/efl_ui_widget_image.h b/src/lib/elementary/efl_ui_widget_image.h index 209d65a..3012c9c 100644 --- a/src/lib/elementary/efl_ui_widget_image.h +++ b/src/lib/elementary/efl_ui_widget_image.h @@ -9,7 +9,6 @@ * IT AT RUNTIME. */ -typedef struct _Async_Open_Data Async_Open_Data; typedef enum { EFL_UI_IMAGE_PRELOAD_ENABLED, @@ -69,9 +68,8 @@ struct _Efl_Ui_Image_Data struct { Ecore_Thread *th; - Async_Open_Data *todo, *done; Eina_Stringshare *file, *key; // only for file_get() - Eina_Spinlock lck; + void *todo; // opaque internal } async; Efl_Ui_Image_Preload_Status preload_status; @@ -95,8 +93,6 @@ struct _Efl_Ui_Image_Data Eina_Bool anim : 1; Eina_Bool play : 1; Eina_Bool async_enable : 1; - Eina_Bool async_opening : 1; - Eina_Bool async_failed : 1; Eina_Bool scale_up : 1; Eina_Bool scale_down : 1; }; --