code review 4830058: test/bench: note changes after recent improvements to l...

test/bench: note changes after recent improvements to locking and runtime
Some tests are significantly faster (50%), a few are slower (up to 30%).
Fannkuch is confusing: parallel code is a little slower for gc, non-parallel and all gccgo runs are faster.

[r, 2011-08-04 01:25:15]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://go.googlecode.com/hg/

[dsymonds, 2011-08-04 01:26:45]

LGTM

[r, 2011-08-04 01:29:34]

*** Submitted as http://code.google.com/p/go/source/detail?r=b4159905ab78 ***

test/bench: note changes after recent improvements to locking and runtime
Some tests are significantly faster (50%), a few are slower (up to 30%).
Fannkuch is confusing: parallel code is a little slower for gc, non-parallel and
all gccgo runs are faster.

R=golang-dev, dsymonds
CC=golang-dev
http://codereview.appspot.com/4830058

[dvyukov, 2011-08-07 11:44:54]

On Thu, Aug 4, 2011 at 5:25 AM, <r@golang.org> wrote:

> Reviewers: golang-dev_googlegroups.com,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://go.googlecode.com/hg/
>
>
> Description:
> test/bench: note changes after recent improvements to locking and
> runtime
> Some tests are significantly faster (50%), a few are slower (up to 30%).
> Fannkuch is confusing: parallel code is a little slower for gc,
> non-parallel and all gccgo runs are faster.
>
> u 0.00s 3.63r # -52%
> +
> +k-nucleotide 1000000
> +       gc k-nucleotide 12.23u 0.02s 12.27r # +27%
> +       gc k-nucleotide-parallel        12.76u 0.02s 4.37r # +29%
> +       gc_B k-nucleotide       12.18u 0.01s 12.21r # +33%
>

That's due to recent changes in runtime.memcpy/memmove:
http://codereview.appspot.com/4813043/

This patch should make the performance even better than it was before. I
expect 10-20% improvement over the previous times:
http://codereview.appspot.com/4815087/

Btw, what is "gc -B"? It seems to be undocumented...

[iant2, 2011-08-07 13:49:54]

FYI

Dmitry Vyukov <dvyukov@google.com> writes:

> Btw, what is "gc -B"? It seems to be undocumented...

It turns off array/slice bounds checking.

Ian

[dvyukov, 2011-08-07 17:59:44]

On Thu, Aug 4, 2011 at 5:25 AM, <r@golang.org> wrote:

>
> Description:
> test/bench: note changes after recent improvements to locking and
> runtime
> Some tests are significantly faster (50%), a few are slower (up to 30%).
> Fannkuch is confusing: parallel code is a little slower for gc,
> non-parallel and all gccgo runs are faster.
> +fannkuch 12
> +       gccgo -O2 fannkuch.go   51.59u 0.00s 51.69r # -4%
> +       gccgo -O2 fannkuch-parallel.go  253.17u 0.00s 64.67r # -11%
> +       gc fannkuch     103.14u 0.00s 103.36r # -5%
> +       gc fannkuch-parallel    189.63u 0.00s 49.37r # +9%
> +       gc_B fannkuch   49.19u 0.00s 49.29r # -14%
>

My guess is that it's aslo caused by slower runtime.memmove. Parallel
version calls runtime.slicecopy, which in turn calls runtime.memove; while
single-threaded version does not call runtime.slicecopy in inner loop AFAIS.
That won't be fixed by my patch.

I looked at the benchmarks before, and I have to admit that
_"unfortunately"_ they are coded in such a way that synchronization related
stuff does not even show up in profiles. So most of my patches should not
affect execution times. The only exception is chameneous-redux... I may try
to beat my C implementation.. It's not going to be all that easy though :)