Remove ArenaPool

It seems not to help performance or memory usage substantially.  Maybe
it does not help at all.

kv.go

@@ -124,7 +124,6 @@ type KV struct {
 	lc        *levelsController
 	vlog      valueLog
 	vptr      valuePointer
-	arenaPool *skl.ArenaPool
 	writeCh   chan *request
 	flushChan chan flushTask // For flushing memtables.
 }
@@ -159,11 +158,10 @@ func NewKV(opt *Options) (out *KV, err error) {
 		flushChan: make(chan flushTask, opt.NumMemtables),
 		writeCh:   make(chan *request, kvWriteChCapacity),
 		opt:       *opt, // Make a copy.
-		arenaPool: skl.NewArenaPool(opt.MaxTableSize+opt.maxBatchSize, opt.NumMemtables+5),
 		closer:    y.NewCloser(),
 		elog:      trace.NewEventLog("Badger", "KV"),
 	}
-	out.mt = skl.NewSkiplist(out.arenaPool)
+	out.mt = skl.NewSkiplist(arenaSize(opt))
 
 	// newLevelsController potentially loads files in directory.
 	if out.lc, err = newLevelsController(out); err != nil {
@@ -893,7 +891,7 @@ func (s *KV) ensureRoomForWrite() error {
 			s.mt.Size(), len(s.flushChan))
 		// We manage to push this task. Let's modify imm.
 		s.imm = append(s.imm, s.mt)
-		s.mt = skl.NewSkiplist(s.arenaPool)
+		s.mt = skl.NewSkiplist(arenaSize(&s.opt))
 		// New memtable is empty. We certainly have room.
 		return nil
 	default:
@@ -902,6 +900,10 @@ func (s *KV) ensureRoomForWrite() error {
 	}
 }
 
+func arenaSize(opt *Options) int64 {
+	return opt.MaxTableSize + opt.maxBatchSize
+}
+
 // WriteLevel0Table flushes memtable. It drops deleteValues.
 func writeLevel0Table(s *skl.Skiplist, f *os.File) error {
 	iter := s.NewIterator()

skl/arena.go

@@ -88,31 +88,3 @@ func (s *Arena) GetVal(offset uint32, size uint16) y.ValueStruct {
 	}
 	return out
 }
-
-type ArenaPool struct {
-	size int64
-	pool chan *Arena // Immutable.
-}
-
-func NewArenaPool(size int64, numArenas int) *ArenaPool {
-	return &ArenaPool{size, make(chan *Arena, numArenas)}
-}
-
-func (s *ArenaPool) Get() *Arena {
-	var arena *Arena
-	select {
-	case arena = <-s.pool:
-	default:
-		arena = NewArena(s.size)
-	}
-	return arena
-}
-
-func (s *ArenaPool) Put(arena *Arena) {
-	arena.Reset()
-	select {
-	case s.pool <- arena:
-	default:
-		// Just ignore the returned arena.
-	}
-}

skl/skl.go

@@ -67,11 +67,10 @@ type node struct {
 }
 
 type Skiplist struct {
-	height    int32 // Current height. 1 <= height <= kMaxHeight. CAS.
-	head      *node
-	ref       int32
-	arena     *Arena
-	arenaPool *ArenaPool
+	height int32 // Current height. 1 <= height <= kMaxHeight. CAS.
+	head   *node
+	ref    int32
+	arena  *Arena
 }
 
 var (
@@ -107,10 +106,9 @@ func (s *Skiplist) DecrRef() {
 		nodePools[len(x.next)].Put(x)
 		x = next
 	}
-	s.arenaPool.Put(s.arena)
-	// Indicate we are closed. Good for testing.
-	// Race condition here would suggest we are accessing skiplist when we are
-	// supposed to have no reference!
+	s.arena.Reset()
+	// Indicate we are closed. Good for testing.  Also, lets GC reclaim memory. Race condition
+	// here would suggest we are accessing skiplist when we are supposed to have no reference!
 	s.arena = nil
 }
 
@@ -128,15 +126,14 @@ func newNode(arena *Arena, key []byte, v y.ValueStruct, height int) *node {
 	}
 }
 
-func NewSkiplist(arenaPool *ArenaPool) *Skiplist {
-	arena := arenaPool.Get()
+func NewSkiplist(arenaSize int64) *Skiplist {
+	arena := NewArena(arenaSize)
 	head := newNode(arena, nil, y.ValueStruct{}, kMaxHeight)
 	return &Skiplist{
-		height:    1,
-		head:      head,
-		arena:     arena,
-		arenaPool: arenaPool,
-		ref:       1,
+		height: 1,
+		head:   head,
+		arena:  arena,
+		ref:    1,
 	}
 }
 

skl/skl_test.go

@@ -30,7 +30,7 @@ import (
 	"github.com/dgraph-io/badger/y"
 )
 
-var arenaPool = NewArenaPool(1<<20, 3)
+const arenaSize = 1 << 20
 
 func newValue(v int) []byte {
 	return []byte(fmt.Sprintf("%05d", v))
@@ -49,7 +49,7 @@ func length(s *Skiplist) int {
 
 func TestEmpty(t *testing.T) {
 	key := []byte("aaa")
-	l := NewSkiplist(arenaPool)
+	l := NewSkiplist(arenaSize)
 
 	v := l.Get(key)
 	require.True(t, v.Value == nil) // Cannot use require.Nil for unsafe.Pointer nil.
@@ -83,7 +83,7 @@ func TestEmpty(t *testing.T) {
 
 // TestBasic tests single-threaded inserts and updates and gets.
 func TestBasic(t *testing.T) {
-	l := NewSkiplist(arenaPool)
+	l := NewSkiplist(arenaSize)
 	val1 := newValue(42)
 	val2 := newValue(52)
 	val3 := newValue(62)
@@ -127,7 +127,7 @@ func TestBasic(t *testing.T) {
 // TestConcurrentBasic tests concurrent writes followed by concurrent reads.
 func TestConcurrentBasic(t *testing.T) {
 	const n = 1000
-	l := NewSkiplist(arenaPool)
+	l := NewSkiplist(arenaSize)
 	var wg sync.WaitGroup
 	for i := 0; i < n; i++ {
 		wg.Add(1)
@@ -157,7 +157,7 @@ func TestConcurrentBasic(t *testing.T) {
 func TestOneKey(t *testing.T) {
 	const n = 100
 	key := []byte("thekey")
-	l := NewSkiplist(arenaPool)
+	l := NewSkiplist(arenaSize)
 	defer l.DecrRef()
 
 	var wg sync.WaitGroup
@@ -191,7 +191,7 @@ func TestOneKey(t *testing.T) {
 }
 
 func TestFindNear(t *testing.T) {
-	l := NewSkiplist(arenaPool)
+	l := NewSkiplist(arenaSize)
 	defer l.DecrRef()
 	for i := 0; i < 1000; i++ {
 		key := fmt.Sprintf("%05d", i*10+5)
@@ -298,7 +298,7 @@ func TestFindNear(t *testing.T) {
 // TestIteratorNext tests a basic iteration over all nodes from the beginning.
 func TestIteratorNext(t *testing.T) {
 	const n = 100
-	l := NewSkiplist(arenaPool)
+	l := NewSkiplist(arenaSize)
 	defer l.DecrRef()
 	it := l.NewIterator()
 	defer it.Close()
@@ -322,7 +322,7 @@ func TestIteratorNext(t *testing.T) {
 // TestIteratorPrev tests a basic iteration over all nodes from the end.
 func TestIteratorPrev(t *testing.T) {
 	const n = 100
-	l := NewSkiplist(arenaPool)
+	l := NewSkiplist(arenaSize)
 	defer l.DecrRef()
 	it := l.NewIterator()
 	defer it.Close()
@@ -347,7 +347,7 @@ func TestIteratorPrev(t *testing.T) {
 // TestIteratorSeek tests Seek and SeekForPrev.
 func TestIteratorSeek(t *testing.T) {
 	const n = 100
-	l := NewSkiplist(arenaPool)
+	l := NewSkiplist(arenaSize)
 	defer l.DecrRef()
 
 	it := l.NewIterator()
@@ -425,7 +425,7 @@ func BenchmarkReadWrite(b *testing.B) {
 	for i := 0; i <= 10; i++ {
 		readFrac := float32(i) / 10.0
 		b.Run(fmt.Sprintf("frac_%d", i), func(b *testing.B) {
-			l := NewSkiplist(arenaPool) // TODO: Fix this. Allow arena size to vary with b.N.
+			l := NewSkiplist(arenaSize) // TODO: Fix this. Allow arena size to vary with b.N.
 			defer l.DecrRef()
 			b.ResetTimer()
 			var count int