Re: [E-devel] [RFC] Caching the result of evas_render_phase1_object_process

[Cedric B. <ced...@fr...>]

On Sun, Apr 13, 2008 at 9:52 PM, Gustavo Sverzut Barbieri
<bar...@pr...> wrote:
> On Sun, Apr 13, 2008 at 1:33 PM, Cedric BAIL <ced...@fr...> wrote:
>  > On Fri, Apr 11, 2008 at 8:56 PM, Cedric BAIL <ced...@fr...> wrote:
>  > >    Some of you already know, I am working on improving the speed of
>  >  >  evas_render_phase1_object_process. The idea is that the list of
>  >  >  active_object, and object to render will change much between to call
>  >  >  to evas_render.
>  >  >
>  >  >    I started by removing the use of evas_list and use a growing array
>  >  >  of evas_object that is only freed on idle. This does bring only a
>  >  >  small speed improvement. Then I started to cache the result
>  >  >  evas_render_phase1_object_process and keep it alive except on layer
>  >  >  change, show, hide and some color set case. On move and resize I
>  >  >  wanted to just check if the state of stack of call to render_pre is
>  >  >  still valid and call them accordingly. It's almost working. Just the
>  >  >  resize is not. I don't understand why yet, but I can't make it work
>  >  >  the same way as the move. Perhaps someone can help me on understanding
>  >  >  this.
>  >  >
>  >  >    So the code is attached for your fun, but don't commit it or it
>  >  >  will break for sure :-)
>  >
>  >  After some sleep I found where was the problem and here is an updated
>  >  patch that finally work. It improve speed when you don't call
>  >  evas_object_show/hide and don't use layer. Each time you call them,
>  >  the cache will be droped in other case it will work.
>  >
>  >  This improvement will complexify the task when we will one day enable
>  >  render_pre for smart object as it make the assumption that no
>  >  evas_object are modified during call to evas_render_updates_internal.
>  >
>  >  Please review this patch and test it with your application.

>  no time to test it now, but an overview is:
>    - good, seems to break nothing (as you said);

Good :-)

>    - my general remarks to your coding style: try to avoid aligning
>  variable declaration, it's not used in E and it causes patches to be
>  dirtier than they need to (changes that just change this). I also
>  dislike the "return ;" (with the space) ;-)
>    - my general remarks about patches: try to avoid useless changes,
>  like changing tabs<->spaces and removing trailing white spaces, do it
>  in separate patch before (now you have commit access, just commit
>  these before you play!)

Right :-)

>    - evas_array seems very useful, make it available to evas as a
>  whole! Just have it to take "void *" and it will do just fine. Also,
>  I'd make the increase amount (16) configurable after the array is
>  created (or provide an alternative constructor).

Good idea, but as this are small functions and some are used a lot I
like the idea of seeing them inlined. As there are more of this kind
of function in Evas, small functions used a lot, I did move them after
some profiling inside src/lib/include/evas_inline.x. This give us a
little bit of speedup with only a small increase in library size. The
function that could be inlined are :
- evas_object_is_opaque
- evas_event_passes_through
- evas_object_is_visible
- evas_object_clippers_is_visible
- evas_object_is_in_output_rect
- evas_object_is_active
- evas_object_coords_recalc
- evas_object_clip_recalc

Are people interested by this kind of patch ? I would add
evas_object_array_push to this list also.

>   - I'd remove the calls to really do things (ie:
>  e->engine.func->output_redraws_rect_del()) from
>  _evas_render_phase1_object_process(), just call
>  _evas_render_phase1_direct() after you built the array... I think this
>  separation will be good and will no cause any damage to performance).
>  OTH, if you want to have performance no matter what, then keep it as
>  is by carry the "should not cache" flag (clean_them), in that case
>  you'd not add to the array at all.

I will look at this option and see if it really impact performance.

>  as you can see, just minor things. It looks very great so far!
>
>  PS: do you have the improvements in numbers? Would rock to present it
>  as a chart, let's also make it public somewhere. (I regret for not
>  doing that for the optimizations I did)

Well as it's a cache mecanism, it doesn't really provide stable
improvement. With expedite as was seeing a score between 490 and 540
on my computer and now it's between 490 and 610. With valgrind
_evas_render_phase1_object_process only take 4% of my total time
compared to the previous use of near 20%. I also think it should
reduce the memory fragmentation of EFL based application, but I don't
have number nor graph for this one.

-- 
Cedric BAIL