[Pixman] [PATCH 07/15] pixman-filter: Speed up the BOX+BOX filter

[Pekka Paalanen]

On Wed, 23 Dec 2015 10:08:01 +0200
Oded Gabbay <oded.gabbay at gmail.com> wrote:

> On Tue, Dec 22, 2015 at 10:53 PM, Bill Spitzak <spitzak at gmail.com> wrote:
> > I think the improvement is obvious if you check how much code is run to do
> > the Simpson's integration. BOX.BOX will literally be the filter used almost
> > always once this is finally fixed.
> >  
> 
> I understand the logic behind this, and I can agree with it, but the
> standard in pixman, AFAIK, is to show actual performance improvements
> using benchmarks/real-world cases.
> 
> I'm not against this patch, and I think it will be eventually merged.
> I'm just saying I think we need to defer it until:
> 
> 1. The changes you talk about them are actually merged into pixman & cairo
> 2. We can show the (hopefully major) improvement using cairo benchmarking.

Hi,

some general comments:

Not to forget, that any piece of existing code you replace should
already have a test excercising it in place, so that when you actually
change the code, the test ensures the effective result is still the
same. (The same does not necessarily imply correct, though. It's just
about catching unintended changes.)

That should be pretty easy to check by deliberately breaking that piece
of code and seeing that 'make check' fails. If necessary, one can use
any of the PIXMAN_DISABLE options to force the use of generic code.

Maybe this is already done and tested, but it would be nice to mention
it, so that the new people who don't know the test suite by heart are
assured it has been considered.

If you find out that the test suite does not cover the code in
question, you may need to extend the test suite first.

This policy would also highlight the cases where old code was actually
wrong and the tests ensure that it stays the same rather than correct.
Fixing this needs even more care as you want to check the new results
really are correct before fixing the test. One might also have to
consider if the old results are relied upon by the users like the Xorg
server, and the fix would break users. In that case the fix cannot land.

Btw. the above is also a warning about changing the meaning of (the
numerical results produced by) existing filter flags. If e.g. Xorg
depends on some flag producing some exact result, you probably can't
change the result of that flag. This would call for looking into Xorg
and X11 test suites.

And Cairo, of course.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/pixman/attachments/20160107/c4efd87a/attachment.sig>