From 63b6f60dcc662eb23603d5028086762ba0f8c171 Mon Sep 17 00:00:00 2001 From: Al Cutter Date: Thu, 16 May 2019 17:47:50 +0100 Subject: [PATCH 1/2] Convert directly from Int.Bits() to NodeID --- storage/types.go | 44 +++++++++++++++++++++++++++++++++++++++++-- storage/types_test.go | 19 +++++++++++++++++-- 2 files changed, 59 insertions(+), 4 deletions(-) diff --git a/storage/types.go b/storage/types.go index d8539d02dc..d5d708f787 100644 --- a/storage/types.go +++ b/storage/types.go @@ -165,10 +165,10 @@ func NewNodeIDFromPrefix(prefix []byte, depth int, index int64, subDepth, totalD } } -// NewNodeIDFromBigInt returns a NodeID of a big.Int with no prefix. +// newNodeIDFromBigIntOld returns a NodeID of a big.Int with no prefix. // index contains the path's least significant bits. // depth indicates the number of bits from the most significant bit to treat as part of the path. -func NewNodeIDFromBigInt(depth int, index *big.Int, totalDepth int) NodeID { +func newNodeIDFromBigIntOld(depth int, index *big.Int, totalDepth int) NodeID { if got, want := totalDepth%8, 0; got != want || got < want { panic(fmt.Sprintf("storage NewNodeFromBitInt(): totalDepth mod 8: %v, want %v", got, want)) } @@ -192,6 +192,46 @@ func NewNodeIDFromBigInt(depth int, index *big.Int, totalDepth int) NodeID { } } +func NewNodeIDFromBigInt(depth int, index *big.Int, totalDepth int) NodeID { + if got, want := totalDepth%8, 0; got != want || got < want { + panic(fmt.Sprintf("storage NewNodeFromBitInt(): totalDepth mod 8: %v, want %v", got, want)) + } + + if totalDepth == 0 { + panic("totalDepth must not be zero") + } + + // Put index in the LSB bits of path. + // This code more-or-less pinched from nat.go in the golang math/big package: + _S := bits.UintSize / 8 + path := make([]byte, totalDepth/8) + + iBits := index.Bits() + i := len(path) +loop: + for _, d := range iBits { + for j := 0; j < _S; j++ { + i-- + if i < 0 { + break loop + } + path[i] = byte(d) + d >>= 8 + } + } + + // TODO(gdbelvin): consider masking off insignificant bits past depth. + if glog.V(5) { + glog.Infof("NewNodeIDFromBigInt(%v, %x, %v): %v, %x", + depth, index, totalDepth, depth, path) + } + + return NodeID{ + Path: path, + PrefixLenBits: depth, + } +} + // BigInt returns the big.Int for this node. func (n NodeID) BigInt() *big.Int { return new(big.Int).SetBytes(n.Path) diff --git a/storage/types_test.go b/storage/types_test.go index 1264d7f375..a88633f047 100644 --- a/storage/types_test.go +++ b/storage/types_test.go @@ -185,7 +185,7 @@ func TestNewNodeIDFromBigInt(t *testing.T) { // We should be able to get the same big.Int back. This is used in // the HStar2 implementation so should be tested. if got, want := n.BigInt(), tc.index; want.Cmp(got) != 0 { - t.Errorf("NewNodeIDFromBigInt(%v, %x, %v): got: %v, want: %v", + t.Errorf("NewNodeIDFromBigInt(%v, %x, %v):got:\n%v, want:\n%v", tc.depth, tc.index.Bytes(), tc.totalDepth, got, want) } } @@ -201,7 +201,7 @@ func TestNewNodeIDFromBigIntPanic(t *testing.T) { defer func() { got := recover() if (got != nil && !want) || (got == nil && want) { - t.Errorf("Incorrect panic behaviour got: %v, want: %v", got, want) + t.Errorf("Incorrect panic behaviour (b=%d) got: %v, want: %v", b, got, want) } }() _ = NewNodeIDFromBigInt(12, big.NewInt(234), b) @@ -892,3 +892,18 @@ func BenchmarkSuffix(b *testing.B) { _ = n.Suffix(10, 176) } } + +func runBenchmarkNewNodeIDFromBigInt(b *testing.B, f func(int, *big.Int, int) NodeID) { + b.Helper() + for i := 0; i < b.N; i++ { + _ = f(256, new(big.Int).SetBytes(h2b("00")), 256) + } +} + +func BenchmarkNewNodeIDFromBigIntOld(b *testing.B) { + runBenchmarkNewNodeIDFromBigInt(b, newNodeIDFromBigIntOld) +} + +func BenchmarkNewNodeIDFromBigIntNew(b *testing.B) { + runBenchmarkNewNodeIDFromBigInt(b, NewNodeIDFromBigInt) +} From 0dea73d1c0467b18a172a841d7583525770efc63 Mon Sep 17 00:00:00 2001 From: Al Cutter Date: Fri, 17 May 2019 16:42:38 +0100 Subject: [PATCH 2/2] fixes --- storage/types.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/storage/types.go b/storage/types.go index d5d708f787..4b28fa09a5 100644 --- a/storage/types.go +++ b/storage/types.go @@ -170,7 +170,7 @@ func NewNodeIDFromPrefix(prefix []byte, depth int, index int64, subDepth, totalD // depth indicates the number of bits from the most significant bit to treat as part of the path. func newNodeIDFromBigIntOld(depth int, index *big.Int, totalDepth int) NodeID { if got, want := totalDepth%8, 0; got != want || got < want { - panic(fmt.Sprintf("storage NewNodeFromBitInt(): totalDepth mod 8: %v, want %v", got, want)) + panic(fmt.Sprintf("storage NewNodeFromBitIntOld(): totalDepth mod 8: %v, want %v", got, want)) } // TODO(al): We _could_ use Bits() and avoid the extra copy/alloc. @@ -182,7 +182,7 @@ func newNodeIDFromBigIntOld(depth int, index *big.Int, totalDepth int) NodeID { // TODO(gdbelvin): consider masking off insignificant bits past depth. if glog.V(5) { - glog.Infof("NewNodeIDFromBigInt(%v, %x, %v): %v, %x", + glog.Infof("NewNodeIDFromBigIntOld(%v, %x, %v): %v, %x", depth, b, totalDepth, depth, path) } @@ -193,17 +193,17 @@ func newNodeIDFromBigIntOld(depth int, index *big.Int, totalDepth int) NodeID { } func NewNodeIDFromBigInt(depth int, index *big.Int, totalDepth int) NodeID { - if got, want := totalDepth%8, 0; got != want || got < want { + if got, want := totalDepth%8, 0; got != want { panic(fmt.Sprintf("storage NewNodeFromBitInt(): totalDepth mod 8: %v, want %v", got, want)) } if totalDepth == 0 { - panic("totalDepth must not be zero") + panic("storage NewNodeFromBitInt(): totalDepth must not be zero") } // Put index in the LSB bits of path. // This code more-or-less pinched from nat.go in the golang math/big package: - _S := bits.UintSize / 8 + const _S = bits.UintSize / 8 path := make([]byte, totalDepth/8) iBits := index.Bits()