From: Josh Bleecher Snyder Date: Tue, 14 Jan 2014 20:54:05 +0000 (-0800) Subject: runtime: change map iteration randomization to use intra-bucket offset X-Git-Tag: go1.3beta1~973 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=3be4d95731a17073afb1f69bde264eecbdfa32bb;p=gostls13.git runtime: change map iteration randomization to use intra-bucket offset Map iteration previously started from a random bucket, but walked each bucket from the beginning. Now, iteration always starts from the first bucket and walks each bucket starting at a random offset. For performance, the random offset is selected at the start of iteration and reused for each bucket. Iteration over a map with 8 or fewer elements--a single bucket--will now be non-deterministic. There will now be only 8 different possible map iterations. Significant benchmark changes, on my OS X laptop (rough but consistent): benchmark old ns/op new ns/op delta BenchmarkMapIter 128 121 -5.47% BenchmarkMapIterEmpty 4.26 4.45 +4.46% BenchmarkNewEmptyMap 114 111 -2.63% Fixes #6719. R=khr, bradfitz CC=golang-codereviews https://golang.org/cl/47370043 --- diff --git a/src/cmd/gc/reflect.c b/src/cmd/gc/reflect.c index 5afcdee489..f54c4cd394 100644 --- a/src/cmd/gc/reflect.c +++ b/src/cmd/gc/reflect.c @@ -252,7 +252,7 @@ hiter(Type *t) // h *Hmap // buckets *Bucket // bptr *Bucket - // other [5]uintptr + // other [4]uintptr // } // must match ../../pkg/runtime/hashmap.c:hash_iter. field[0] = typ(TFIELD); @@ -289,8 +289,8 @@ hiter(Type *t) field[6] = typ(TFIELD); field[6]->type = typ(TARRAY); field[6]->type->type = types[TUINTPTR]; - field[6]->type->bound = 5; - field[6]->type->width = 5 * widthptr; + field[6]->type->bound = 4; + field[6]->type->width = 4 * widthptr; field[6]->sym = mal(sizeof(Sym)); field[6]->sym->name = "other"; @@ -306,8 +306,8 @@ hiter(Type *t) } field[6]->down = T; off += field[6]->type->width; - if(off != 11 * widthptr) - yyerror("hash_iter size not correct %d %d", off, 11 * widthptr); + if(off != 10 * widthptr) + yyerror("hash_iter size not correct %d %d", off, 10 * widthptr); t->hiter = i; i->map = t; return i; diff --git a/src/pkg/runtime/hashmap.c b/src/pkg/runtime/hashmap.c index 101c4281f6..c359e2a14d 100644 --- a/src/pkg/runtime/hashmap.c +++ b/src/pkg/runtime/hashmap.c @@ -746,9 +746,8 @@ struct hash_iter byte *buckets; // bucket ptr at hash_iter initialization time struct Bucket *bptr; // current bucket - // end point for iteration - uintptr endbucket; - bool wrapped; + uint32 offset; // intra-bucket offset to start from during iteration + bool done; // state of table at time iterator is initialized uint8 B; @@ -768,8 +767,8 @@ hash_iter_init(MapType *t, Hmap *h, struct hash_iter *it) { uint32 old; - if(sizeof(struct hash_iter) / sizeof(uintptr) != 11) { - runtime·throw("hash_iter size incorrect"); // see ../../cmd/gc/range.c + if(sizeof(struct hash_iter) / sizeof(uintptr) != 10) { + runtime·throw("hash_iter size incorrect"); // see ../../cmd/gc/reflect.c } it->t = t; it->h = h; @@ -779,8 +778,9 @@ hash_iter_init(MapType *t, Hmap *h, struct hash_iter *it) it->buckets = h->buckets; // iterator state - it->bucket = it->endbucket = runtime·fastrand1() & (((uintptr)1 << h->B) - 1); - it->wrapped = false; + it->bucket = 0; + it->offset = runtime·fastrand1() & (BUCKETSIZE - 1); + it->done = false; it->bptr = nil; // Remember we have an iterator. @@ -795,8 +795,8 @@ hash_iter_init(MapType *t, Hmap *h, struct hash_iter *it) if(h->buckets == nil) { // Empty map. Force next hash_next to exit without - // evalulating h->bucket. - it->wrapped = true; + // evaluating h->bucket. + it->done = true; } } @@ -810,7 +810,7 @@ hash_next(struct hash_iter *it) uintptr bucket, oldbucket; uintptr hash; Bucket *b; - uintptr i; + uintptr i, offi; intptr check_bucket; bool eq; byte *k, *v; @@ -825,7 +825,7 @@ hash_next(struct hash_iter *it) next: if(b == nil) { - if(bucket == it->endbucket && it->wrapped) { + if(it->done) { // end of iteration it->key = nil; it->value = nil; @@ -851,14 +851,15 @@ next: bucket++; if(bucket == ((uintptr)1 << it->B)) { bucket = 0; - it->wrapped = true; + it->done = true; } i = 0; } - k = b->data + h->keysize * i; - v = b->data + h->keysize * BUCKETSIZE + h->valuesize * i; - for(; i < BUCKETSIZE; i++, k += h->keysize, v += h->valuesize) { - if(b->tophash[i] != Empty && b->tophash[i] != EvacuatedEmpty) { + for(; i < BUCKETSIZE; i++) { + offi = (i + it->offset) & (BUCKETSIZE - 1); + k = b->data + h->keysize * offi; + v = b->data + h->keysize * BUCKETSIZE + h->valuesize * offi; + if(b->tophash[offi] != Empty && b->tophash[offi] != EvacuatedEmpty) { if(check_bucket >= 0) { // Special case: iterator was started during a grow and the // grow is not done yet. We're working on a bucket whose @@ -884,12 +885,12 @@ next: // NOTE: this case is why we need two evacuate tophash // values, evacuatedX and evacuatedY, that differ in // their low bit. - if(check_bucket >> (it->B - 1) != (b->tophash[i] & 1)) { + if(check_bucket >> (it->B - 1) != (b->tophash[offi] & 1)) { continue; } } } - if(b->tophash[i] != EvacuatedX && b->tophash[i] != EvacuatedY) { + if(b->tophash[offi] != EvacuatedX && b->tophash[offi] != EvacuatedY) { // this is the golden data, we can return it. it->key = IK(h, k); it->value = IV(h, v); diff --git a/src/pkg/runtime/map_test.go b/src/pkg/runtime/map_test.go index b8586483fd..9c703ba362 100644 --- a/src/pkg/runtime/map_test.go +++ b/src/pkg/runtime/map_test.go @@ -411,8 +411,7 @@ func TestMapNanGrowIterator(t *testing.T) { } func TestMapIterOrder(t *testing.T) { - // TODO: For issue 6719, add 3 and 7 to this list. - for _, n := range [...]int{9, 15} { + for _, n := range [...]int{3, 7, 9, 15} { // Make m be {0: true, 1: true, ..., n-1: true}. m := make(map[int]bool) for i := 0; i < n; i++ {