chess icon indicating copy to clipboard operation
chess copied to clipboard

Avoid string operations/allocation in newBitboard

Open tov opened this issue 4 years ago • 2 comments

Instead of building a string of '0's and '1' characters and then converting it, newBitboard uses bit arithmetic to convert a map to a uint64. This avoids unnecessary memory allocation and string processing.

tov avatar Jun 11 '21 18:06 tov

@tov could you add a test for this change? Also I am not sure this is performance sensitive code. What motivated this change?

notnil avatar Sep 08 '21 22:09 notnil

@notnil, here's a test (sorry :)

diff --git a/bitboard_test.go b/bitboard_test.go
index 0000000..3c0a55f 100644
--- a/bitboard_test.go
+++ b/bitboard_test.go
@@ -7,6 +7,11 @@ type bitboardTestPair struct {
 	reversed uint64
 }
 
+type bitboardTestNew struct {
+	smap map[Square]bool
+	bits uint64
+}
+
 var (
 	tests = []bitboardTestPair{
 		{
@@ -22,6 +27,50 @@ var (
 			uint64(0),
 		},
 	}
+	newBitboardTests = []bitboardTestNew{
+		{
+			map[Square]bool{},
+			0b_00000000_00000000_00000000_00000000_00000000_00000000_00000000_00000000,
+			// all zeroes
+		},
+		{
+			map[Square]bool{
+				A1: true,
+			},
+			0b_10000000_00000000_00000000_00000000_00000000_00000000_00000000_00000000,
+			// ^
+		},
+		{
+			map[Square]bool{
+				B1: true,
+			},
+			0b_01000000_00000000_00000000_00000000_00000000_00000000_00000000_00000000,
+			//  ^
+		},
+		{
+			map[Square]bool{
+				B3: true,
+			},
+			0b_00000000_00000000_01000000_00000000_00000000_00000000_00000000_00000000,
+			//                    ^
+		},
+		{
+			map[Square]bool{
+				H8: true,
+			},
+			0b_00000000_00000000_00000000_00000000_00000000_00000000_00000000_00000001,
+			//                                                                       ^
+		},
+		{
+			map[Square]bool{
+				A1: true,
+				B3: true,
+				H8: true,
+			},
+			0b_10000000_00000000_01000000_00000000_00000000_00000000_00000000_00000001,
+			// ^                  ^                                                  ^
+		},
+	}
 )
 
 func TestBitboardReverse(t *testing.T) {
@@ -53,3 +102,12 @@ func BenchmarkBitboardReverse(b *testing.B) {
 func intStr(i uint64) string {
 	return bitboard(i).String()
 }
+
+func TestBitboardNew(t *testing.T) {
+	for _, c := range newBitboardTests {
+		bb := newBitboard(c.smap)
+		if uint64(bb) != c.bits {
+			t.Fatalf("new bitboard from %v expected %s but got %s", c.smap, intStr(c.bits), bb)
+		}
+	}
+}

bbars avatar Jul 12 '22 12:07 bbars