On Fri, Jun 6, 2008 at 4:26 PM, Gustavo Sverzut Barbieri
<[EMAIL PROTECTED]> wrote:
> On Fri, Jun 6, 2008 at 6:05 AM, Cedric BAIL <[EMAIL PROTECTED]> wrote:
>> With the last serie of patch I committed inside evas, evas_render
>> should now be working again correctly. This fixed version only give
>> around 7% of speedup to expedite test and some are a little bit slower
>> than the previous.
>>
>> So after diging a little bit more the slower tests case of expedite, I
>> think the attached patch will make all expedite test as fast as
>> without the cache inside evas_render and give a global 10% speedup to
>> expedite without any cache.
>>
>> The idea is to remove as much allocation and list manipulation as
>> possible by implementing a special array of Evas_Rectangle. This patch
>> work for me on all my application, E17 and edje old test application.
>>
>> So as always have fun reviewing this patch !
>
>
> Looks good in general, just think we should use inverted names like
> the rest of evas:
>
> evas_add_rect -> evas_rectangles_add()
Right.
> - obj->clip.changes = updates;
> + for (i = 0; i < rects->count; ++i)
> + {
> + r = malloc(sizeof(Evas_Rectangle));
> + if (!r) goto end;
> +
> + *r = rects->array[i];
> + obj->clip.changes = evas_list_append(obj->clip.changes, r);
> + }
>
> no way to avoid this? Why not change obj->clip.changes to be a pointer
> to rects? This way we save this extra walk and lots of small malloc()
> and overhead of lists (memory fragmentation, extra pointer for
> "->data" and no evas_memorypool...)... also eliminating the need to
> free the rects->array right below:
In fact before changing this I did run expedite with a printf to see
if this code was called... and I never saw the printf. So I didn't
took the time to improve this :-)
> + end:
> + free(rects->array);
> + rects->array = NULL;
> + rects->count = 0;
> + rects->total = 0;
>
> Just remember that rects might be a pointer in the stack, if so you
> need to malloc() + memcpy() it.
Yes, that why I don't free(rects) and just the array.
> + if (obj->smart.smart) return ;
> + if (is_v == was_v) return ;
>
> why do you like this space between "return" and ";" ? :-P
:-D
> +/* if (tmp.count > 0) */
> +/* { */
> +/* unsigned int i; */
> +
> +/* for (i = 0; i < tmp.count; ++i) */
> +/* { */
> +/* if ((tmp.array[i].w > 0) && (tmp.array[i].h > 0)) */
> +/* { */
> +/* int intsec1, intsec2; */
> +
> +/* intsec1 = (RECTS_INTERSECT(tmp.array[i].x,
> tmp.array[i].y, tmp.array[i].w, tmp.array[i].h, x, y, w, h)); */
> +/* intsec2 = (RECTS_INTERSECT(tmp.array[i].x,
> tmp.array[i].y, tmp.array[i].w, tmp.array[i].h, xx, yy, ww, hh)); */
> +/* if (intsec1 ^ intsec2) */
> +/* { */
> +/* evas_add_rect(rects, tmp.array[i].x,
> tmp.array[i].y,
> tmp.array[i].w, tmp.array[i].h); */
> +/* } */
> +/* } */
> +/* } */
> +/* free(tmp.array); */
> +/* } */
>
> what's this? remove or uncomment?
Because I didn't really understood why it needed two list in the
previous version. So I keep it around just in case.
> + if ((rects->count + 1) > rects->total)
> + {
> + Evas_Rectangle *_add_rect;
> + unsigned int _tmp_total;
> +
> + _tmp_total = rects->total + 32;
> + _add_rect = realloc(rects->array, sizeof(Evas_Rectangle) *
> _tmp_total);
> + if (!_add_rect) return ;
> +
> + rects->total = _tmp_total;
> + rects->array = _add_rect;
>
> and
>
> + if (rects->max < (rects->active + 1))
> + {
> + rects->max += 32;
> + rects->rects = realloc(rects->rects, sizeof(Cutout_Rect) *
> rects->max);
> + }
>
> "so close, yet so far...". Make them "almost the same", the second
> version doesn't handle any error. also change the comparison to be
> uniform.
You already saw the next discussion I would like to start ! :-)
Cutout_Rect are really the same as Evas_Rectangles in their definition
and they are allocated/reallocated a lot between two call to
evas_render. The same for Evas_Rectangles. In fact, they are the main
source of call to malloc/realloc and in some expedite test, it's
consuming around 3 to 5% of the application time.
Of course the first thing we should do is to merge both
Evas_Rectangles and Cutout_Rects.
But the most important improvement would be to keep all this
Cutout_Rects and Evas_Rectangles arrays around between to call to
evas_render and we could flush them only when needed. I just don't
know where to keep them.
Oh, and if someone know how to improve
evas_common_draw_context_cutout_split, it's consume around 20% of
expedite time in many tests.
--
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
