refactor: refact listpack

xgzlucario · Jul 11, 2024 · 0d28823 · 0d28823
1 parent 2bdf210
commit 0d28823
Show file tree

Hide file tree

Showing 6 changed files with 145 additions and 134 deletions.
diff --git a/command.go b/command.go
@@ -318,9 +318,8 @@ func lrangeCommand(writer *RESPWriter, args []RESP) {
 	}
 
 	writer.WriteArrayHead(size)
-	ls.Range(start, end, func(data []byte) (stop bool) {
+	ls.Range(start, end, func(data []byte) {
 		writer.WriteBulk(data)
-		return false
 	})
 }
 

diff --git a/internal/list/bench_test.go b/internal/list/bench_test.go
@@ -4,35 +4,48 @@ import (
 	"testing"
 )
 
+func BenchmarkList(b *testing.B) {
+	b.Run("lpush", func(b *testing.B) {
+		ls := New()
+		for i := 0; i < b.N; i++ {
+			ls.LPush(genKey(i))
+		}
+	})
+	b.Run("rpush", func(b *testing.B) {
+		ls := New()
+		for i := 0; i < b.N; i++ {
+			ls.RPush(genKey(i))
+		}
+	})
+}
+
 func BenchmarkListPack(b *testing.B) {
 	const N = 1000
 	b.Run("next", func(b *testing.B) {
 		ls := genListPack(0, N)
 		it := ls.NewIterator()
 		b.ResetTimer()
 		for i := 0; i < b.N; i++ {
-			it.SeekBegin()
-			it.Next()
+			it.SeekBegin().Next()
 		}
 	})
 	b.Run("prev", func(b *testing.B) {
 		ls := genListPack(0, N)
 		it := ls.NewIterator()
 		b.ResetTimer()
 		for i := 0; i < b.N; i++ {
-			it.SeekEnd()
-			it.Prev()
+			it.SeekEnd().Prev()
 		}
 	})
 	b.Run("lpush", func(b *testing.B) {
 		lp := NewListPack()
-		for i := 0; i < 99999; i++ {
+		for i := 0; i < 10*10000; i++ {
 			lp.LPush("A")
 		}
 	})
 	b.Run("rpush", func(b *testing.B) {
 		lp := NewListPack()
-		for i := 0; i < 99999; i++ {
+		for i := 0; i < 10*10000; i++ {
 			lp.RPush("A")
 		}
 	})

diff --git a/internal/list/list.go b/internal/list/list.go
@@ -1,5 +1,7 @@
 package list
 
+import "math"
+
 //	 +------------------------------ QuickList -----------------------------+
 //	 |	     +-----------+     +-----------+             +-----------+      |
 //	head --- | listpack0 | <-> | listpack1 | <-> ... <-> | listpackN | --- tail
@@ -76,46 +78,43 @@ func (ls *QuickList) RPop() (key string, ok bool) {
 
 // free release empty list node.
 func (ls *QuickList) free(n *Node) {
-	if n.size == 0 && n.prev != nil && n.next != nil {
+	if n.prev != nil && n.next != nil {
 		n.prev.next = n.next
 		n.next.prev = n.prev
 		bpool.Put(n.data)
 		n = nil
 	}
 }
 
-// Size
 func (ls *QuickList) Size() (n int) {
 	for lp := ls.head; lp != nil; lp = lp.next {
 		n += lp.Size()
 	}
 	return
 }
 
-// Index
-func (ls *QuickList) Index(index int) (string, bool) {
+type lsIterator func(data []byte)
+
+func (ls *QuickList) Range(start, end int, f lsIterator) {
+	if end == -1 {
+		end = math.MaxInt
+	}
 	for lp := ls.head; lp != nil; lp = lp.next {
-		if index > lp.Size() {
-			index -= lp.Size()
-		} else {
-			it := lp.NewIterator()
-			var data []byte
-			for index >= 0 {
-				data = it.Next()
-				index--
-			}
-			return string(data), true
+		it := lp.NewIterator().SeekBegin()
+		for !it.IsEnd() {
+			f(it.Next())
 		}
 	}
-	return "", false
-}
-
-type lsIterator func(data []byte) (stop bool)
-
-// Range
-func (ls *QuickList) Range(start, end int, f lsIterator) {
 }
 
-// RevRange
 func (ls *QuickList) RevRange(start, end int, f lsIterator) {
+	if end == -1 {
+		end = math.MaxInt
+	}
+	for lp := ls.tail; lp != nil; lp = lp.prev {
+		it := lp.NewIterator().SeekEnd()
+		for !it.IsBegin() {
+			f(it.Prev())
+		}
+	}
 }
diff --git a/internal/list/list_test.go b/internal/list/list_test.go
@@ -1,6 +1,7 @@
 package list
 
 import (
+	"slices"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -14,43 +15,40 @@ func genList(start, end int) *QuickList {
 	return lp
 }
 
+func list2slice(ls *QuickList) (res []string) {
+	ls.Range(0, -1, func(data []byte) {
+		res = append(res, string(data))
+	})
+	return
+}
+
 func TestList(t *testing.T) {
 	const N = 1000
 	assert := assert.New(t)
 	SetMaxListPackSize(128)
 
-	t.Run("rpush", func(t *testing.T) {
+	t.Run("lpush", func(t *testing.T) {
 		ls := New()
+		ls2 := make([]string, 0, N)
 		for i := 0; i < N; i++ {
-			assert.Equal(ls.Size(), i)
-			ls.RPush(genKey(i))
-		}
-		for i := 0; i < N; i++ {
-			v, ok := ls.Index(i)
-			assert.Equal(genKey(i), v)
-			assert.Equal(true, ok)
-		}
-		// check each node length
-		for cur := ls.head; cur != nil; cur = cur.next {
-			assert.LessOrEqual(len(cur.data), maxListPackSize)
+			key := genKey(i)
+			ls.LPush(key)
+			ls2 = slices.Insert(ls2, 0, key)
 		}
+		assert.Equal(ls.Size(), len(ls2))
+		assert.Equal(list2slice(ls), ls2)
 	})
 
-	t.Run("lpush", func(t *testing.T) {
+	t.Run("rpush", func(t *testing.T) {
 		ls := New()
+		ls2 := make([]string, 0, N)
 		for i := 0; i < N; i++ {
-			assert.Equal(ls.Size(), i)
-			ls.LPush(genKey(i))
-		}
-		for i := 0; i < N; i++ {
-			v, ok := ls.Index(N - 1 - i)
-			assert.Equal(genKey(i), v)
-			assert.Equal(true, ok)
-		}
-		// check each node length
-		for cur := ls.head; cur != nil; cur = cur.next {
-			assert.LessOrEqual(len(cur.data), maxListPackSize)
+			key := genKey(i)
+			ls.RPush(key)
+			ls2 = append(ls2, key)
 		}
+		assert.Equal(ls.Size(), len(ls2))
+		assert.Equal(list2slice(ls), ls2)
 	})
 
 	t.Run("lpop", func(t *testing.T) {
@@ -81,57 +79,21 @@ func TestList(t *testing.T) {
 		assert.Equal(false, ok)
 	})
 
-	t.Run("len", func(t *testing.T) {
-		ls := New()
-		for i := 0; i < N; i++ {
-			ls.RPush(genKey(i))
-			assert.Equal(ls.Size(), i+1)
-		}
-	})
-
 	t.Run("range", func(t *testing.T) {
-		ls := New()
-		ls.Range(1, 2, func(s []byte) bool {
-			panic("should not call")
-		})
-		ls = genList(0, N)
-
-		var count int
-		ls.Range(0, -1, func(s []byte) bool {
-			assert.Equal(string(s), genKey(count))
-			count++
-			return false
-		})
-		assert.Equal(count, N)
-
-		ls.Range(1, 1, func(s []byte) bool {
-			panic("should not call")
-		})
-		ls.Range(-1, -1, func(s []byte) bool {
-			panic("should not call")
+		ls := genList(0, N)
+		i := 0
+		ls.Range(0, -1, func(data []byte) {
+			assert.Equal(string(data), genKey(i))
+			i++
 		})
 	})
 
 	t.Run("revrange", func(t *testing.T) {
-		ls := New()
-		ls.RevRange(1, 2, func(s []byte) bool {
-			panic("should not call")
-		})
-		ls = genList(0, N)
-
-		var count int
-		ls.RevRange(0, -1, func(s []byte) bool {
-			assert.Equal(string(s), genKey(N-count-1))
-			count++
-			return false
-		})
-		assert.Equal(count, N)
-
-		ls.RevRange(1, 1, func(s []byte) bool {
-			panic("should not call")
-		})
-		ls.RevRange(-1, -1, func(s []byte) bool {
-			panic("should not call")
+		ls := genList(0, N)
+		i := 0
+		ls.RevRange(0, -1, func(data []byte) {
+			assert.Equal(string(data), genKey(N-i-1))
+			i++
 		})
 	})
 }
diff --git a/internal/list/listpack.go b/internal/list/listpack.go
@@ -35,7 +35,7 @@ type ListPack struct {
 }
 
 func NewListPack() *ListPack {
-	return &ListPack{data: make([]byte, 0, 32)}
+	return &ListPack{data: bpool.Get(32)[:0]}
 }
 
 func (lp *ListPack) Size() int {
@@ -51,17 +51,11 @@ func (lp *ListPack) RPush(data ...string) {
 }
 
 func (lp *ListPack) LPop() (string, bool) {
-	if lp.size == 0 {
-		return "", false
-	}
-	return lp.NewIterator().RemoveNext(), true
+	return lp.NewIterator().RemoveNext()
 }
 
 func (lp *ListPack) RPop() (string, bool) {
-	if lp.size == 0 {
-		return "", false
-	}
-	return lp.NewIterator().SeekEnd().RemovePrev(), true
+	return lp.NewIterator().SeekEnd().RemovePrev()
 }
 
 type lpIterator struct {
@@ -141,23 +135,29 @@ func (it *lpIterator) Insert(datas ...string) {
 	bpool.Put(alloc)
 }
 
-func (it *lpIterator) RemoveNext() string {
+func (it *lpIterator) RemoveNext() (string, bool) {
+	if it.IsEnd() {
+		return "", false
+	}
 	before := it.index
-	data := string(it.Next())
+	data := string(it.Next()) // seek to next
 	after := it.index
 	it.data = slices.Delete(it.data, before, after)
-	it.index = before
+	it.index = before // back to prev
 	it.size--
-	return data
+	return data, true
 }
 
-func (it *lpIterator) RemovePrev() string {
+func (it *lpIterator) RemovePrev() (string, bool) {
+	if it.IsBegin() {
+		return "", false
+	}
 	before := it.index
-	data := string(it.Prev())
+	data := string(it.Prev()) // seek to prev
 	after := it.index
 	it.data = slices.Delete(it.data, after, before)
 	it.size--
-	return data
+	return data, true
 }
 
 func appendEntry(dst []byte, data string) []byte {