From 38e3ce10fe626a257c4b9fcfa2ffe421923a5c71 Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Fri, 31 Aug 2018 10:32:33 -0500 Subject: [PATCH 1/5] removing bounds check --- roaring/roaring.go | 103 +++++++++++++++++++++++-------- roaring/roaring_internal_test.go | 4 +- 2 files changed, 78 insertions(+), 29 deletions(-) diff --git a/roaring/roaring.go b/roaring/roaring.go index e7333325c..c39c8b2e0 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -2136,15 +2136,26 @@ func intersectArrayBitmap(a, b *Container) *Container { } func intersectBitmapBitmap(a, b *Container) *Container { - output := &Container{bitmap: make([]uint64, bitmapN), containerType: containerBitmap} + var ( - for i := range a.bitmap { - v := a.bitmap[i] & b.bitmap[i] - output.bitmap[i] = v - output.n += int(popcount(v)) + ab = a.bitmap[:bitmapN] + bb = b.bitmap[:bitmapN] + buf = make([]uint64, bitmapN) + ob =buf[:bitmapN] + n int +) + for i := 0; i < bitmapN; i++ { + v := ab[i] & bb[i] + ob[i] = v + n += int(popcount(v)) + } + + output := &Container{ + bitmap: ob, + n: n, + containerType: containerBitmap, } - output.optimize() return output } @@ -2434,17 +2445,26 @@ func unionArrayBitmap(a, b *Container) *Container { } func unionBitmapBitmap(a, b *Container) *Container { - output := &Container{ - bitmap: make([]uint64, bitmapN), - containerType: containerBitmap, - } + var ( + ab = a.bitmap[:bitmapN] + bb = b.bitmap[:bitmapN] + buf = make([]uint64, bitmapN) + ob =buf[:bitmapN] + + n int + ) for i := 0; i < bitmapN; i++ { - v := a.bitmap[i] | b.bitmap[i] - output.bitmap[i] = v - output.n += int(popcount(v)) + v := ab[i] | bb[i] + ob[i] = v + n += int(popcount(v)) } + output := &Container{ + bitmap: ob, + n: n, + containerType: containerBitmap, + } return output } @@ -2780,13 +2800,25 @@ func differenceBitmapArray(a, b *Container) *Container { } func differenceBitmapBitmap(a, b *Container) *Container { - output := &Container{bitmap: make([]uint64, bitmapN), containerType: containerBitmap} + var ( + ab = a.bitmap[:bitmapN] + bb = b.bitmap[:bitmapN] + buf = make([]uint64, bitmapN) + ob =buf[:bitmapN] - for i := range a.bitmap { - v := a.bitmap[i] & (^b.bitmap[i]) - output.bitmap[i] = v - output.n += int(popcount(v)) + n int + ) + for i := 0; i < bitmapN; i++ { + v := ab[i] & (^bb[i]) + ob[i] = v + n += int(popcount(v)) + } + + output := &Container{ + bitmap: ob, + n: n, + containerType: containerBitmap, } if output.n < ArrayMaxSize { output.bitmapToArray() @@ -2871,16 +2903,26 @@ func xorArrayBitmap(a, b *Container) *Container { } func xorBitmapBitmap(a, b *Container) *Container { - output := &Container{ - bitmap: make([]uint64, bitmapN), - containerType: containerBitmap, - } + var ( + ab = a.bitmap[:bitmapN] + bb = b.bitmap[:bitmapN] + buf = make([]uint64, bitmapN) + ob =buf[:bitmapN] + + n int + ) + for i := 0; i < bitmapN; i++ { - v := a.bitmap[i] ^ b.bitmap[i] - output.bitmap[i] = v - output.n += int(popcount(v)) + v := ab[i] ^ bb[i] + ob[i] = v + n += int(popcount(v)) } + output := &Container{ + bitmap: ob, + n: n, + containerType: containerBitmap, + } if output.count() < ArrayMaxSize { output.bitmapToArray() } @@ -3333,9 +3375,16 @@ func popcount(x uint64) uint64 { } func popcountAndSlice(s, m []uint64) uint64 { + var ( + a=s[:bitmapN] + b=m[:bitmapN] + ) + _ = a[bitmapN-1] + _ = b[bitmapN-1] + cnt := uint64(0) - for i := range s { - cnt += popcount(s[i] & m[i]) + for i:=0;i Date: Fri, 14 Sep 2018 10:27:33 -0500 Subject: [PATCH 2/5] updated comments and gofmt --- roaring/roaring.go | 67 ++++++++++++++++++++++++++-------------------- 1 file changed, 38 insertions(+), 29 deletions(-) diff --git a/roaring/roaring.go b/roaring/roaring.go index c39c8b2e0..c98c13621 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -2136,15 +2136,15 @@ func intersectArrayBitmap(a, b *Container) *Container { } func intersectBitmapBitmap(a, b *Container) *Container { + // local variables added to prevent BCE checks in loop + // see https://go101.org/article/bounds-check-elimination.html var ( - - ab = a.bitmap[:bitmapN] - bb = b.bitmap[:bitmapN] - buf = make([]uint64, bitmapN) - ob =buf[:bitmapN] - - n int -) + ab = a.bitmap[:bitmapN] + bb = b.bitmap[:bitmapN] + buf = make([]uint64, bitmapN) + ob = buf[:bitmapN] + n int + ) for i := 0; i < bitmapN; i++ { v := ab[i] & bb[i] ob[i] = v @@ -2152,8 +2152,8 @@ func intersectBitmapBitmap(a, b *Container) *Container { } output := &Container{ - bitmap: ob, - n: n, + bitmap: ob, + n: n, containerType: containerBitmap, } return output @@ -2445,11 +2445,14 @@ func unionArrayBitmap(a, b *Container) *Container { } func unionBitmapBitmap(a, b *Container) *Container { + // local variables added to prevent BCE checks in loop + // see https://go101.org/article/bounds-check-elimination.html + var ( - ab = a.bitmap[:bitmapN] - bb = b.bitmap[:bitmapN] + ab = a.bitmap[:bitmapN] + bb = b.bitmap[:bitmapN] buf = make([]uint64, bitmapN) - ob =buf[:bitmapN] + ob = buf[:bitmapN] n int ) @@ -2461,8 +2464,8 @@ func unionBitmapBitmap(a, b *Container) *Container { } output := &Container{ - bitmap: ob, - n: n, + bitmap: ob, + n: n, containerType: containerBitmap, } return output @@ -2800,11 +2803,14 @@ func differenceBitmapArray(a, b *Container) *Container { } func differenceBitmapBitmap(a, b *Container) *Container { + // local variables added to prevent BCE checks in loop + // see https://go101.org/article/bounds-check-elimination.html + var ( - ab = a.bitmap[:bitmapN] - bb = b.bitmap[:bitmapN] + ab = a.bitmap[:bitmapN] + bb = b.bitmap[:bitmapN] buf = make([]uint64, bitmapN) - ob =buf[:bitmapN] + ob = buf[:bitmapN] n int ) @@ -2816,8 +2822,8 @@ func differenceBitmapBitmap(a, b *Container) *Container { } output := &Container{ - bitmap: ob, - n: n, + bitmap: ob, + n: n, containerType: containerBitmap, } if output.n < ArrayMaxSize { @@ -2903,11 +2909,14 @@ func xorArrayBitmap(a, b *Container) *Container { } func xorBitmapBitmap(a, b *Container) *Container { + // local variables added to prevent BCE checks in loop + // see https://go101.org/article/bounds-check-elimination.html + var ( - ab = a.bitmap[:bitmapN] - bb = b.bitmap[:bitmapN] + ab = a.bitmap[:bitmapN] + bb = b.bitmap[:bitmapN] buf = make([]uint64, bitmapN) - ob =buf[:bitmapN] + ob = buf[:bitmapN] n int ) @@ -2919,8 +2928,8 @@ func xorBitmapBitmap(a, b *Container) *Container { } output := &Container{ - bitmap: ob, - n: n, + bitmap: ob, + n: n, containerType: containerBitmap, } if output.count() < ArrayMaxSize { @@ -3376,14 +3385,14 @@ func popcount(x uint64) uint64 { func popcountAndSlice(s, m []uint64) uint64 { var ( - a=s[:bitmapN] - b=m[:bitmapN] + a = s[:bitmapN] + b = m[:bitmapN] ) _ = a[bitmapN-1] _ = b[bitmapN-1] - + cnt := uint64(0) - for i:=0;i Date: Fri, 14 Sep 2018 14:46:02 -0500 Subject: [PATCH 3/5] code cleanup --- roaring/roaring.go | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/roaring/roaring.go b/roaring/roaring.go index 14be4ea0b..87b6eac3d 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -2148,9 +2148,8 @@ func intersectBitmapBitmap(a, b *Container) *Container { n int ) for i := 0; i < bitmapN; i++ { - v := ab[i] & bb[i] - ob[i] = v - n += int(popcount(v)) + ob[i] = ab[i] & bb[i] + n += int(popcount(ob[i])) } output := &Container{ @@ -2460,9 +2459,8 @@ func unionBitmapBitmap(a, b *Container) *Container { ) for i := 0; i < bitmapN; i++ { - v := ab[i] | bb[i] - ob[i] = v - n += int(popcount(v)) + ob[i] =ab[i] | bb[i] + n += int(popcount(ob[i])) } output := &Container{ @@ -2818,9 +2816,8 @@ func differenceBitmapBitmap(a, b *Container) *Container { ) for i := 0; i < bitmapN; i++ { - v := ab[i] & (^bb[i]) - ob[i] = v - n += int(popcount(v)) + ob[i] = ab[i] & (^bb[i]) + n += int(popcount(ob[i])) } output := &Container{ @@ -2924,9 +2921,8 @@ func xorBitmapBitmap(a, b *Container) *Container { ) for i := 0; i < bitmapN; i++ { - v := ab[i] ^ bb[i] - ob[i] = v - n += int(popcount(v)) + ob[i] = ab[i] ^ bb[i] + n += int(popcount(ob[i])) } output := &Container{ From fe8756927cd6e6bcd9bf4b57e8a90ba944750b9f Mon Sep 17 00:00:00 2001 From: Todd Gruben Date: Fri, 14 Sep 2018 15:40:37 -0500 Subject: [PATCH 4/5] manual gofmt --- roaring/roaring.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/roaring/roaring.go b/roaring/roaring.go index 87b6eac3d..5e82364b1 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -2459,7 +2459,7 @@ func unionBitmapBitmap(a, b *Container) *Container { ) for i := 0; i < bitmapN; i++ { - ob[i] =ab[i] | bb[i] + ob[i] = ab[i] | bb[i] n += int(popcount(ob[i])) } From 3ca7944fe33f6422789372ddd82c94a0b0c1636c Mon Sep 17 00:00:00 2001 From: Matt Jaffee Date: Thu, 20 Sep 2018 12:37:28 -0500 Subject: [PATCH 5/5] remove unecessary lines confirmed that bounds checks are still avoided by go test -gcflags="-d=ssa/check_bce/debug=1" ./roaring.go:3387:8: Found IsSliceInBounds ./roaring.go:3388:8: Found IsSliceInBounds ./roaring.go:3436:21: Found IsSliceInBounds --- roaring/roaring.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/roaring/roaring.go b/roaring/roaring.go index 4a5eea5fc..6eaad740d 100644 --- a/roaring/roaring.go +++ b/roaring/roaring.go @@ -3387,8 +3387,6 @@ func popcountAndSlice(s, m []uint64) uint64 { a = s[:bitmapN] b = m[:bitmapN] ) - _ = a[bitmapN-1] - _ = b[bitmapN-1] cnt := uint64(0) for i := 0; i < bitmapN; i++ {