Merge pull request #169 from benbjohnson/allocation

Fix freelist allocation direction.

9 files changed, 78 insertions, 56 deletions

diff --git a/Makefile b/Makefile
index 2c0dc97..67635ae 100644
--- a/Makefile
+++ b/Makefile
@@ -19,7 +19,7 @@ cover: fmt
 
 cpuprofile: fmt
 	@go test -c
-	@./bolt.test -test.v -test.run="^X" -test.bench=$(BENCH) -test.cpuprofile cpu.prof
+	@./bolt.test -test.v -test.run=$(TEST) -test.cpuprofile cpu.prof
 
 # go get github.com/kisielk/errcheck
 errcheck:
diff --git a/bucket_test.go b/bucket_test.go
index f6bd13f..16172a5 100644
--- a/bucket_test.go
+++ b/bucket_test.go
@@ -899,30 +899,21 @@ func TestBucket_Delete_Quick(t *testing.T) {
 			assert.NoError(t, err)
 
 			// Remove items one at a time and check consistency.
-			for i, item := range items {
+			for _, item := range items {
 				err := db.Update(func(tx *Tx) error {
 					return tx.Bucket([]byte("widgets")).Delete(item.Key)
 				})
 				assert.NoError(t, err)
+			}
 
-				// Anything before our deletion index should be nil.
-				db.View(func(tx *Tx) error {
-					b := tx.Bucket([]byte("widgets"))
-					for j, exp := range items {
-						value := b.Get(exp.Key)
-						if j > i {
-							if !assert.Equal(t, exp.Value, value) {
-								t.FailNow()
-							}
-						} else {
-							if !assert.Nil(t, value) {
-								t.FailNow()
-							}
-						}
-					}
+			// Anything before our deletion index should be nil.
+			db.View(func(tx *Tx) error {
+				tx.Bucket([]byte("widgets")).ForEach(func(k, v []byte) error {
+					t.Fatalf("bucket should be empty; found: %06x", trunc(k, 3))
 					return nil
 				})
-			}
+				return nil
+			})
 		})
 		return true
 	}
diff --git a/cursor.go b/cursor.go
index 31a3d5f..1c14d05 100644
--- a/cursor.go
+++ b/cursor.go
@@ -193,7 +193,7 @@ func (c *Cursor) last() {
 func (c *Cursor) search(key []byte, pgid pgid) {
 	p, n := c.bucket.pageNode(pgid)
 	if p != nil {
-		_assert((p.flags&(branchPageFlag|leafPageFlag)) != 0, "invalid page type: %d: %s", p.id, p.typ())
+		_assert((p.flags&(branchPageFlag|leafPageFlag)) != 0, "invalid page type: %d: %x", p.id, p.flags)
 	}
 	e := elemRef{page: p, node: n}
 	c.stack = append(c.stack, e)
diff --git a/db.go b/db.go
index 1a1f997..5b6cc3e 100644
--- a/db.go
+++ b/db.go
@@ -350,7 +350,6 @@ func (db *DB) beginTx() (*Tx, error) {
 
 	// Lock the meta pages while we initialize the transaction.
 	db.metalock.Lock()
-	defer db.metalock.Unlock()
 
 	// Exit if the database is not open yet.
 	if !db.opened {
@@ -364,6 +363,16 @@ func (db *DB) beginTx() (*Tx, error) {
 
 	// Keep track of transaction until it closes.
 	db.txs = append(db.txs, t)
+	n := len(db.txs)
+
+	// Unlock the meta pages.
+	db.metalock.Unlock()
+
+	// Update the transaction stats.
+	db.statlock.Lock()
+	db.stats.TxN++
+	db.stats.OpenTxN = n
+	db.statlock.Unlock()
 
 	return t, nil
 }
@@ -417,12 +426,14 @@ func (db *DB) removeTx(tx *Tx) {
 			break
 		}
 	}
+	n := len(db.txs)
 
 	// Unlock the meta pages.
 	db.metalock.Unlock()
 
 	// Merge statistics.
 	db.statlock.Lock()
+	db.stats.OpenTxN = n
 	db.stats.TxStats.add(&tx.stats)
 	db.statlock.Unlock()
 }
@@ -552,6 +563,10 @@ func (db *DB) allocate(count int) (*page, error) {
 
 // Stats represents statistics about the database.
 type Stats struct {
+	// Transaction stats
+	TxN     int // total number of completed read transactions
+	OpenTxN int // number of currently open read transactions
+
 	TxStats TxStats // global, ongoing stats.
 }
 
diff --git a/db_test.go b/db_test.go
index 7b1f554..b85f8cb 100644
--- a/db_test.go
+++ b/db_test.go
@@ -295,10 +295,10 @@ func TestDB_Consistency(t *testing.T) {
 				assert.Equal(t, "free", p.Type)
 			}
 			if p, _ := tx.Page(4); assert.NotNil(t, p) {
-				assert.Equal(t, "freelist", p.Type)
+				assert.Equal(t, "leaf", p.Type) // root leaf
 			}
 			if p, _ := tx.Page(5); assert.NotNil(t, p) {
-				assert.Equal(t, "leaf", p.Type) // root leaf
+				assert.Equal(t, "freelist", p.Type)
 			}
 			p, _ := tx.Page(6)
 			assert.Nil(t, p)
diff --git a/freelist.go b/freelist.go
index ebe2810..149e595 100644
--- a/freelist.go
+++ b/freelist.go
@@ -26,30 +26,42 @@ func (f *freelist) all() []pgid {
 		ids = append(ids, list...)
 	}
 
-	sort.Sort(reverseSortedPgids(ids))
+	sort.Sort(pgids(ids))
 	return ids
 }
 
 // allocate returns the starting page id of a contiguous list of pages of a given size.
 // If a contiguous block cannot be found then 0 is returned.
 func (f *freelist) allocate(n int) pgid {
-	var count int
-	var previd pgid
+	if len(f.ids) == 0 {
+		return 0
+	}
+
+	var initial, previd pgid
 	for i, id := range f.ids {
-		// Reset count if this is not contiguous.
-		if previd == 0 || previd-id != 1 {
-			count = 1
+		_assert(id > 1, "invalid page allocation: %d", id)
+
+		// Reset initial page if this is not contiguous.
+		if previd == 0 || id-previd != 1 {
+			initial = id
 		}
 
 		// If we found a contiguous block then remove it and return it.
-		if count == n {
-			f.ids = append(f.ids[:i-(n-1)], f.ids[i+1:]...)
-			_assert(id > 1, "cannot allocate page 0 or 1: %d", id)
-			return id
+		if (id-initial)+1 == pgid(n) {
+			// If we're allocating off the beginning then take the fast path
+			// and just adjust the existing slice. This will use extra memory
+			// temporarily but the append() in free() will realloc the slice
+			// as is necessary.
+			if (i + 1) == n {
+				f.ids = f.ids[i+1:]
+			} else {
+				copy(f.ids[i-n+1:], f.ids[i+1:])
+				f.ids = f.ids[:len(f.ids)-n]
+			}
+			return initial
 		}
 
 		previd = id
-		count++
 	}
 	return 0
 }
@@ -74,7 +86,7 @@ func (f *freelist) release(txid txid) {
 			delete(f.pending, tid)
 		}
 	}
-	sort.Sort(reverseSortedPgids(f.ids))
+	sort.Sort(pgids(f.ids))
 }
 
 // isFree returns whether a given page is in the free list.
@@ -99,6 +111,7 @@ func (f *freelist) read(p *page) {
 	ids := ((*[maxAllocSize]pgid)(unsafe.Pointer(&p.ptr)))[0:p.count]
 	f.ids = make([]pgid, len(ids))
 	copy(f.ids, ids)
+	sort.Sort(pgids(f.ids))
 }
 
 // write writes the page ids onto a freelist page. All free and pending ids are
@@ -133,9 +146,3 @@ func (f *freelist) check() {
 	}
 }
 */
-
-type reverseSortedPgids []pgid
-
-func (s reverseSortedPgids) Len() int           { return len(s) }
-func (s reverseSortedPgids) Swap(i, j int)      { s[i], s[j] = s[j], s[i] }
-func (s reverseSortedPgids) Less(i, j int) bool { return s[i] > s[j] }
diff --git a/freelist_test.go b/freelist_test.go
index 2b321a4..5948f3b 100644
--- a/freelist_test.go
+++ b/freelist_test.go
@@ -29,22 +29,25 @@ func TestFreelist_release(t *testing.T) {
 	f.free(102, &page{id: 39})
 	f.release(100)
 	f.release(101)
-	assert.Equal(t, f.ids, []pgid{13, 12, 9})
+	assert.Equal(t, []pgid{9, 12, 13}, f.ids)
 	f.release(102)
-	assert.Equal(t, f.ids, []pgid{39, 13, 12, 9})
+	assert.Equal(t, []pgid{9, 12, 13, 39}, f.ids)
 }
 
 // Ensure that a freelist can find contiguous blocks of pages.
 func TestFreelist_allocate(t *testing.T) {
-	f := &freelist{ids: []pgid{18, 13, 12, 9, 7, 6, 5, 4, 3}}
-	assert.Equal(t, f.allocate(2), pgid(12))
-	assert.Equal(t, f.allocate(1), pgid(18))
-	assert.Equal(t, f.allocate(3), pgid(5))
-	assert.Equal(t, f.allocate(3), pgid(0))
-	assert.Equal(t, f.allocate(2), pgid(3))
-	assert.Equal(t, f.allocate(1), pgid(9))
-	assert.Equal(t, f.allocate(0), pgid(0))
-	assert.Equal(t, f.ids, []pgid{})
+	f := &freelist{ids: []pgid{3, 4, 5, 6, 7, 9, 12, 13, 18}}
+	assert.Equal(t, 3, int(f.allocate(3)))
+	assert.Equal(t, 6, int(f.allocate(1)))
+	assert.Equal(t, 0, int(f.allocate(3)))
+	assert.Equal(t, 12, int(f.allocate(2)))
+	assert.Equal(t, 7, int(f.allocate(1)))
+	assert.Equal(t, 0, int(f.allocate(0)))
+	assert.Equal(t, []pgid{9, 18}, f.ids)
+	assert.Equal(t, 9, int(f.allocate(1)))
+	assert.Equal(t, 18, int(f.allocate(1)))
+	assert.Equal(t, 0, int(f.allocate(1)))
+	assert.Equal(t, []pgid{}, f.ids)
 }
 
 // Ensure that a freelist can deserialize from a freelist page.
@@ -87,9 +90,9 @@ func TestFreelist_write(t *testing.T) {
 	// Ensure that the freelist is correct.
 	// All pages should be present and in reverse order.
 	assert.Equal(t, len(f2.ids), 5)
-	assert.Equal(t, f2.ids[0], pgid(39))
-	assert.Equal(t, f2.ids[1], pgid(28))
+	assert.Equal(t, f2.ids[0], pgid(3))
+	assert.Equal(t, f2.ids[1], pgid(11))
 	assert.Equal(t, f2.ids[2], pgid(12))
-	assert.Equal(t, f2.ids[3], pgid(11))
-	assert.Equal(t, f2.ids[4], pgid(3))
+	assert.Equal(t, f2.ids[3], pgid(28))
+	assert.Equal(t, f2.ids[4], pgid(39))
 }
diff --git a/page.go b/page.go
index 56cf064..78ca898 100644
--- a/page.go
+++ b/page.go
@@ -128,3 +128,9 @@ type PageInfo struct {
 	Count         int
 	OverflowCount int
 }
+
+type pgids []pgid
+
+func (s pgids) Len() int           { return len(s) }
+func (s pgids) Swap(i, j int)      { s[i], s[j] = s[j], s[i] }
+func (s pgids) Less(i, j int) bool { return s[i] < s[j] }
diff --git a/tx_test.go b/tx_test.go
index 05e7466..f6ee3d3 100644
--- a/tx_test.go
+++ b/tx_test.go
@@ -219,7 +219,7 @@ func TestTx_DeleteBucket(t *testing.T) {
 
 		db.Update(func(tx *Tx) error {
 			// Verify that the bucket's page is free.
-			assert.Equal(t, []pgid{5, 4}, db.freelist.all())
+			assert.Equal(t, []pgid{4, 5}, db.freelist.all())
 
 			// Create the bucket again and make sure there's not a phantom value.
 			b, err := tx.CreateBucket([]byte("widgets"))