code review 107340047: go.image/vp8: use branch-free abs

go.image/vp8: use branch-free abs

This change seems to restore performance to
the level before CL 108140045.

benchmark                         old ns/op    new ns/op    delta
BenchmarkDecodeVP8SimpleFilter       796416       784394   -1.51%
BenchmarkDecodeVP8NormalFilter      4931138      4693078   -4.83%
BenchmarkDecodeVP8L                 7820030      7821796   +0.02%

[ruiu, 2014-06-27 00:14:20]

Hello nigeltao@golang.org (cc: golang-codereviews@googlegroups.com),

I'd like you to review this change to
https://ruiu%40google.com@code.google.com/p/go.image/

[nigeltao, 2014-06-27 00:46:45]

LGTM if r is OK with this size-of-int bit-shiftery.

[nigeltao, 2014-06-27 00:48:29]

BTW another idea is to change all the "int"s in filter.go to "int32"s. It might
be worth re-running the benchmarks with that change too.

[r, 2014-06-27 04:46:11]

LGTM with tweaks
leaving for nigeltao

https://codereview.appspot.com/107340047/diff/80001/vp8/filter.go
File vp8/filter.go (right):

https://codereview.appspot.com/107340047/diff/80001/vp8/filter.go#newcode230
vp8/filter.go:230: const intSize = 32 << uint(^uint(0)>>63)
this needs a little explanation and there is a needless conversion

// intSize is either 32 or 64.
const intSize = 32 << (^uint(0)>>63)

https://codereview.appspot.com/107340047/diff/80001/vp8/filter.go#newcode239
vp8/filter.go:239: // code with a branch.
// See Hacker's Delight, section 2-4.

[ruiu, 2014-06-27 23:40:51]

I tried to change int to int32 as Nigel suggested.
The benchmark results were the same as this patch.
I'll keep them as they are.

https://codereview.appspot.com/107340047/diff/80001/vp8/filter.go
File vp8/filter.go (right):

https://codereview.appspot.com/107340047/diff/80001/vp8/filter.go#newcode230
vp8/filter.go:230: const intSize = 32 << uint(^uint(0)>>63)
On 2014/06/27 04:46:11, r wrote:
> this needs a little explanation and there is a needless conversion
> 
> // intSize is either 32 or 64.
> const intSize = 32 << (^uint(0)>>63)
> 

Done.

https://codereview.appspot.com/107340047/diff/80001/vp8/filter.go#newcode239
vp8/filter.go:239: // code with a branch.
On 2014/06/27 04:46:11, r wrote:
> // See Hacker's Delight, section 2-4.

Done.

[r, 2014-06-28 00:52:26]

LGTM

[r, 2014-06-28 00:53:17]

(still leaving for nige)

[nigeltao, 2014-06-30 00:33:49]

*** Submitted as
https://code.google.com/p/go/source/detail?r=f6e41288d919&repo=image ***

go.image/vp8: use branch-free abs

This change seems to restore performance to
the level before CL 108140045.

benchmark                         old ns/op    new ns/op    delta
BenchmarkDecodeVP8SimpleFilter       796416       784394   -1.51%
BenchmarkDecodeVP8NormalFilter      4931138      4693078   -4.83%
BenchmarkDecodeVP8L                 7820030      7821796   +0.02%

LGTM=nigeltao, r
R=nigeltao, r
CC=golang-codereviews
https://codereview.appspot.com/107340047

Committer: Nigel Tao <nigeltao@golang.org>