code review 6215078: runtime: handle and test large map values

runtime: handle and test large map values

This is from CL 5451105 but was dropped from that CL.
See also CL 6137051.

The only change compared to 5451105 is to check for
h != nil in reflect·mapiterinit; allowing use of nil maps
must have happened after that original CL.

Fixes issue 3573.

[rsc, 2012-05-24 21:54:42]

Hello golang-dev@googlegroups.com,

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

[dave_cheney.net, 2012-05-24 22:38:15]

hi rsc,

this CL may cause a regression for small key types. 

linux/amd64 atom n455 @ 1.66ghz


lucky(~/go/src/pkg/runtime) % ~/go/misc/benchcmp {old,new}.txt
benchmark                 old ns/op    new ns/op    delta
BenchmarkMapUint8Get1           307          317   +3.26%
BenchmarkMapUint8Get2           315          329   +4.44%
BenchmarkMapUint8Put            383          413   +7.83%
BenchmarkMapUint16Get1          349          378   +8.31%
BenchmarkMapUint16Get2          354          390  +10.17%
BenchmarkMapUint16Put           421          466  +10.69%
BenchmarkMapUint32Get1          391          391   +0.00%
BenchmarkMapUint32Get2          396          435   +9.85%
BenchmarkMapUint32Put           457          482   +5.47%
BenchmarkMapUint64Get1          460          469   +1.96%
BenchmarkMapUint64Get2          465          483   +3.87%
BenchmarkMapUint64Put           525          555   +5.71%

source for benchmarks is https://gist.github.com/c382a1c0a723b81a2fff.

[dave_cheney.net, 2012-05-24 22:40:59]

for comparison, here are the numbers on a 5g panda board

benchmark                 old ns/op    new ns/op    delta
BenchmarkMapUint8Get1           685          715   +4.38%
BenchmarkMapUint8Get2           732          724   -1.09%
BenchmarkMapUint8Put            809          840   +3.83%
BenchmarkMapUint16Get1          703          719   +2.28%
BenchmarkMapUint16Get2          717          744   +3.77%
BenchmarkMapUint16Put           805          826   +2.61%
BenchmarkMapUint32Get1          712          731   +2.67%
BenchmarkMapUint32Get2          726          734   +1.10%
BenchmarkMapUint32Put           841          854   +1.55%
BenchmarkMapUint64Get1          767          796   +3.78%
BenchmarkMapUint64Get2          782          809   +3.45%
BenchmarkMapUint64Put           880          905   +2.84%

[r, 2012-05-24 23:13:43]

LGTM

http://codereview.appspot.com/6215078/diff/8001/src/pkg/runtime/hashmap.c
File src/pkg/runtime/hashmap.c (right):

http://codereview.appspot.com/6215078/diff/8001/src/pkg/runtime/hashmap.c#new...
src/pkg/runtime/hashmap.c:9: #define IndirectVal  (1<<0)	/* storing pointers to
values */
/* Hmap flags */

[rsc, 2012-05-25 02:38:05]

On Thu, May 24, 2012 at 6:38 PM,  <dave@cheney.net> wrote:
> BenchmarkMapUint32Get2          396          435   +9.85%

i see about +1% on my laptop.  i'd like to understand it, but
i don't see anything obvious, and in any event correctness
beats performance.  if someone wants to try to track it down,
that'd be fine with me.  the chips are complex.

russ

[rsc, 2012-05-25 02:41:15]

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

runtime: handle and test large map values

This is from CL 5451105 but was dropped from that CL.
See also CL 6137051.

The only change compared to 5451105 is to check for
h != nil in reflect·mapiterinit; allowing use of nil maps
must have happened after that original CL.

Fixes issue 3573.

R=golang-dev, dave, r
CC=golang-dev
http://codereview.appspot.com/6215078