[Pixman] [PATCH 0/18] Add iterators to images

[Siarhei Siamashka]

On Thursday 06 January 2011 04:09:38 Søren Sandmann wrote:
> The following patch series changes the scanline access to be based on
> iterators instead of direct calls to virtual functions. There are
> several benefits to this:
> 
> - Since destination iterators are different from source iterators, we
>   can fix the bug we have currently, where destination transformations
>   and filters are not being ignored as they should.
> 
> - The annoying "classify" virtual functions can be eliminated.
> 
> Both of those are fixed in the patch series. There are some other
> potential benefits:
> 
> - The initialization of the iterators is virtualized, so that a CPU
>   specific implementation can plug in its own fetcher. This means we
>   can have SIMD accelerated gradients and fetchers.
> 
> - Fetchers for NONE-extended images would be able to reuse the
>   generated scanlines for the initial and final zero-lines, and could
>   avoid reinitializing the zero parts of most scanlines if there is
>   no transformation.
> 
> - A hypothetical polygon image couldn't support random access to
>   scanlines, so it would need something like this, where the scanlines
>   are guaranteed to be fetched in order.
> 
> None of these are done yet, though.
> 
> Comments appreciated.

I have some mixed feelings about this patchset.

One one hand, adding iterators is a very good idea. When looking from the
lowest level stuff, in order to get good performance we need to avoid doing 
anything redundant (intermediate buffers, multiple passes, recalculating
something multiple times, long chains of calls, etc.) and implement the really
needed things in an efficient way (using SIMD, cache friendly memory access
patterns, etc.).

Currently the images (as they go into simple fast path functions) are 
represented by the following set: pointer to the first pixel to be processed, 
width, height, stride. This set can also cover some of the extra operations,
including vertical flipping of the image (by using negative stride) and
horizontal linear gradients (by using zero stride).

Providing an additional way of representing images using iterators in the fast
path functions can help to easily adapt them to also support more types of
source images such as scaled images or gradients. Thus increasing the subset
of operations which are well optimized and work fast in pixman. And this is
great.

>  15 files changed, 743 insertions(+), 470 deletions(-)

On the other hand, I'm not so happy about the implementation. The number of
lines of code as well as the complexity has increased, and the performance
seems to have dropped a little bit in some of the cases. I really want to
see some practical benefits which depend on these added iterators in the
near future.

cairo-perf-trace of pixman git head (a484a9c49c98dfad0d74af4440039f61bef24d48)
compiled with MMX/SSE2 optimizations disabled because we want to see how the
performance of the general path is affected:

[ # ]  backend                         test   min(s) median(s) stddev. count
[ # ]    image: pixman 0.21.3
[  0]    image                      poppler    3.660    3.679   0.30%    6/6
[ # ]    image: pixman 0.21.3
[  1]    image            xfce4-terminal-a1    3.725    3.730   0.06%    6/6
[ # ]    image: pixman 0.21.3
[  2]    image         gnome-system-monitor    4.738    4.740   0.12%    5/6
[ # ]    image: pixman 0.21.3
[  3]    image                    ocitysmap    2.772    2.774   0.09%    6/6
[ # ]    image: pixman 0.21.3
[  4]    image                    evolution    9.446    9.459   0.09%    6/6
[ # ]    image: pixman 0.21.3
[  5]    image           swfdec-giant-steps    2.724    2.731   0.20%    6/6
[ # ]    image: pixman 0.21.3
[  6]    image            firefox-talos-gfx   13.499   13.499   0.05%    5/6
[ # ]    image: pixman 0.21.3
[  7]    image                midori-zoomed    2.912    2.918   0.09%    6/6
[ # ]    image: pixman 0.21.3
[  8]    image         firefox-planet-gnome   15.800   16.005   0.59%    6/6
[ # ]    image: pixman 0.21.3
[  9]    image                         gvim   13.968   13.977   0.04%    5/6
[ # ]    image: pixman 0.21.3
[ 10]    image            firefox-talos-svg   50.940   52.027   0.90%    6/6
[ # ]    image: pixman 0.21.3
[ 11]    image               swfdec-youtube    4.210    4.213   0.06%    6/6
[ # ]    image: pixman 0.21.3
[ 12]    image           gnome-terminal-vim    3.162    3.168   0.12%    6/6

All the same benchmark with the patches from 'iterators4' branch applied:

[ # ]  backend                         test   min(s) median(s) stddev. count
[ # ]    image: pixman 0.21.3
[  0]    image                      poppler    3.680    3.685   0.08%    6/6
[ # ]    image: pixman 0.21.3
[  1]    image            xfce4-terminal-a1    3.713    3.715   0.04%    6/6
[ # ]    image: pixman 0.21.3
[  2]    image         gnome-system-monitor    4.855    4.858   0.03%    5/6
[ # ]    image: pixman 0.21.3
[  3]    image                    ocitysmap    2.759    2.761   0.04%    6/6
[ # ]    image: pixman 0.21.3
[  4]    image                    evolution    9.459    9.472   0.06%    6/6
[ # ]    image: pixman 0.21.3
[  5]    image           swfdec-giant-steps    2.752    2.755   0.06%    6/6
[ # ]    image: pixman 0.21.3
[  6]    image            firefox-talos-gfx   13.445   13.454   0.04%    6/6
[ # ]    image: pixman 0.21.3
[  7]    image                midori-zoomed    2.935    2.938   0.05%    6/6
[ # ]    image: pixman 0.21.3
[  8]    image         firefox-planet-gnome   16.168   16.184   0.11%    6/6
[ # ]    image: pixman 0.21.3
[  9]    image                         gvim   14.266   14.277   0.05%    6/6
[ # ]    image: pixman 0.21.3
[ 10]    image            firefox-talos-svg   51.503   51.605   0.08%    5/6
[ # ]    image: pixman 0.21.3
[ 11]    image               swfdec-youtube    4.223    4.224   0.03%    6/6
[ # ]    image: pixman 0.21.3
[ 12]    image           gnome-terminal-vim    3.226    3.232   0.08%    6/6


-- 
Best regards,
Siarhei Siamashka