Decompose the interface

trying to decouple the interface a bit further, inversing the interface injection again. Signed-off-by: Thomas Jungblut <[email protected]>
etcd-io · Jul 25, 2024 · c4c0414 · c4c0414
1 parent 2e6b37b
commit c4c0414
Show file tree

Hide file tree

Showing 5 changed files with 80 additions and 66 deletions.
diff --git a/internal/freelist/array.go b/internal/freelist/array.go
@@ -8,17 +8,14 @@ import (
 )
 
 type array struct {
-	*shared
-
 	ids []common.Pgid // all free and available free page ids.
 }
 
 func (f *array) Init(ids common.Pgids) {
 	f.ids = ids
-	f.reindex()
 }
 
-func (f *array) Allocate(txid common.Txid, n int) common.Pgid {
+func (f *array) alloc(txid common.Txid, n int, allocs *map[common.Pgid]common.Txid, cache *map[common.Pgid]struct{}) common.Pgid {
 	if len(f.ids) == 0 {
 		return 0
 	}
@@ -49,9 +46,9 @@ func (f *array) Allocate(txid common.Txid, n int) common.Pgid {
 
 			// Remove from the free cache.
 			for i := common.Pgid(0); i < common.Pgid(n); i++ {
-				delete(f.cache, initial+i)
+				delete(*cache, initial+i)
 			}
-			f.allocs[initial] = txid
+			(*allocs)[initial] = txid
 			return initial
 		}
 
@@ -99,9 +96,5 @@ func (f *array) mergeSpans(ids common.Pgids) {
 }
 
 func NewArrayFreelist() Interface {
-	a := &array{
-		shared: newShared(),
-	}
-	a.Interface = a
-	return a
+	return newShared(&array{})
 }
diff --git a/internal/freelist/freelist.go b/internal/freelist/freelist.go
@@ -16,26 +16,24 @@ type ReadWriter interface {
 	EstimatedWritePageSize() int
 }
 
-type Interface interface {
-	ReadWriter
-
+type allocator interface {
 	// Init initializes this freelist with the given list of pages.
 	Init(ids common.Pgids)
 
-	// Allocate tries to allocate the given number of contiguous pages
-	// from the free list pages. It returns the starting page ID if
-	// available; otherwise, it returns 0.
-	Allocate(txid common.Txid, numPages int) common.Pgid
-
-	// Count returns the number of free and pending pages.
-	Count() int
-
 	// FreeCount returns the number of free pages.
 	FreeCount() int
 
-	// PendingCount returns the number of pending pages.
-	PendingCount() int
+	// freePageIds returns the IDs of all free pages.
+	freePageIds() common.Pgids
 
+	// mergeSpans is merging the given pages into the freelist
+	mergeSpans(ids common.Pgids)
+
+	// TODO(thomas): this is necessary to decouple, but leaks internals
+	alloc(txid common.Txid, numPages int, allocs *map[common.Pgid]common.Txid, cache *map[common.Pgid]struct{}) common.Pgid
+}
+
+type txManager interface {
 	// AddReadonlyTXID adds a given read-only transaction id for pending page tracking.
 	AddReadonlyTXID(txid common.Txid)
 
@@ -45,6 +43,32 @@ type Interface interface {
 	// ReleasePendingPages releases any pages associated with closed read-only transactions.
 	ReleasePendingPages()
 
+	// pendingPageIds returns all pending pages by transaction id.
+	pendingPageIds() map[common.Txid]*txPending
+
+	// release moves all page ids for a transaction id (or older) to the freelist.
+	release(txId common.Txid)
+
+	// releaseRange moves pending pages allocated within an extent [begin,end] to the free list.
+	releaseRange(begin, end common.Txid)
+}
+
+type Interface interface {
+	ReadWriter
+	allocator
+	txManager
+
+	// Allocate tries to allocate the given number of contiguous pages
+	// from the free list pages. It returns the starting page ID if
+	// available; otherwise, it returns 0.
+	Allocate(txid common.Txid, numPages int) common.Pgid
+
+	// Count returns the number of free and pending pages.
+	Count() int
+
+	// PendingCount returns the number of pending pages.
+	PendingCount() int
+
 	// Free releases a page and its overflow for a given transaction id.
 	// If the page is already free then a panic will occur.
 	Free(txId common.Txid, p *common.Page)
@@ -64,19 +88,4 @@ type Interface interface {
 
 	// NoSyncReload reads the freelist from Pgids and filters out pending items.
 	NoSyncReload(pgIds common.Pgids)
-
-	// freePageIds returns the IDs of all free pages.
-	freePageIds() common.Pgids
-
-	// pendingPageIds returns all pending pages by transaction id.
-	pendingPageIds() map[common.Txid]*txPending
-
-	// release moves all page ids for a transaction id (or older) to the freelist.
-	release(txId common.Txid)
-
-	// releaseRange moves pending pages allocated within an extent [begin,end] to the free list.
-	releaseRange(begin, end common.Txid)
-
-	// mergeSpans is merging the given pages into the freelist
-	mergeSpans(ids common.Pgids)
 }
diff --git a/internal/freelist/hashmap.go b/internal/freelist/hashmap.go
@@ -12,8 +12,6 @@ import (
 type pidSet map[common.Pgid]struct{}
 
 type hashMap struct {
-	*shared
-
 	freePagesCount uint64                 // count of free pages(hashmap version)
 	freemaps       map[uint64]pidSet      // key is the size of continuous pages(span), value is a set which contains the starting pgids of same size
 	forwardMap     map[common.Pgid]uint64 // key is start pgid, value is its span size
@@ -54,11 +52,9 @@ func (f *hashMap) Init(pgids common.Pgids) {
 	if size != 0 && start != 0 {
 		f.addSpan(start, size)
 	}
-
-	f.reindex()
 }
 
-func (f *hashMap) Allocate(txid common.Txid, n int) common.Pgid {
+func (f *hashMap) alloc(txid common.Txid, n int, allocs *map[common.Pgid]common.Txid, cache *map[common.Pgid]struct{}) common.Pgid {
 	if n == 0 {
 		return 0
 	}
@@ -69,10 +65,10 @@ func (f *hashMap) Allocate(txid common.Txid, n int) common.Pgid {
 			// remove the span
 			f.delSpan(pid, uint64(n))
 
-			f.allocs[pid] = txid
+			(*allocs)[pid] = txid
 
 			for i := common.Pgid(0); i < common.Pgid(n); i++ {
-				delete(f.cache, pid+i)
+				delete(*cache, pid+i)
 			}
 			return pid
 		}
@@ -88,15 +84,15 @@ func (f *hashMap) Allocate(txid common.Txid, n int) common.Pgid {
 			// remove the initial
 			f.delSpan(pid, size)
 
-			f.allocs[pid] = txid
+			(*allocs)[pid] = txid
 
 			remain := size - uint64(n)
 
 			// add remain span
 			f.addSpan(pid+common.Pgid(n), remain)
 
 			for i := common.Pgid(0); i < common.Pgid(n); i++ {
-				delete(f.cache, pid+i)
+				delete(*cache, pid+i)
 			}
 			return pid
 		}
@@ -280,13 +276,14 @@ func (f *hashMap) idsFromBackwardMap() map[common.Pgid]struct{} {
 	return ids
 }
 
-func NewHashMapFreelist() Interface {
-	hm := &hashMap{
-		shared:      newShared(),
+func newHashMap() *hashMap {
+	return &hashMap{
 		freemaps:    make(map[uint64]pidSet),
 		forwardMap:  make(map[common.Pgid]uint64),
 		backwardMap: make(map[common.Pgid]uint64),
 	}
-	hm.Interface = hm
-	return hm
+}
+
+func NewHashMapFreelist() Interface {
+	return newShared(newHashMap())
 }
diff --git a/internal/freelist/hashmap_test.go b/internal/freelist/hashmap_test.go
@@ -86,7 +86,7 @@ func TestFreelistHashmap_mergeWithExist(t *testing.T) {
 		},
 	}
 	for _, tt := range tests {
-		f := newTestHashMapFreelist()
+		f := newHashMap()
 		f.Init(tt.ids)
 
 		f.mergeWithExistingSpan(tt.pgid)
@@ -107,7 +107,7 @@ func TestFreelistHashmap_mergeWithExist(t *testing.T) {
 }
 
 func TestFreelistHashmap_GetFreePageIDs(t *testing.T) {
-	f := newTestHashMapFreelist()
+	f := newHashMap()
 
 	N := int32(100000)
 	fm := make(map[common.Pgid]uint64)
@@ -129,7 +129,7 @@ func TestFreelistHashmap_GetFreePageIDs(t *testing.T) {
 }
 
 func Benchmark_freelist_hashmapGetFreePageIDs(b *testing.B) {
-	f := newTestHashMapFreelist()
+	f := newHashMap()
 	N := int32(100000)
 	fm := make(map[common.Pgid]uint64)
 	i := int32(0)
@@ -148,8 +148,3 @@ func Benchmark_freelist_hashmapGetFreePageIDs(b *testing.B) {
 		f.freePageIds()
 	}
 }
-
-func newTestHashMapFreelist() *hashMap {
-	f := NewHashMapFreelist()
-	return f.(*hashMap)
-}
diff --git a/internal/freelist/shared.go b/internal/freelist/shared.go
@@ -16,22 +16,32 @@ type txPending struct {
 }
 
 type shared struct {
-	Interface
+	allocator
 
 	readonlyTXIDs []common.Txid               // all readonly transaction IDs.
 	allocs        map[common.Pgid]common.Txid // mapping of Txid that allocated a pgid.
 	cache         map[common.Pgid]struct{}    // fast lookup of all free and pending page ids.
 	pending       map[common.Txid]*txPending  // mapping of soon-to-be free page ids by tx.
 }
 
-func newShared() *shared {
+func newShared(a allocator) Interface {
 	return &shared{
-		pending: make(map[common.Txid]*txPending),
-		allocs:  make(map[common.Pgid]common.Txid),
-		cache:   make(map[common.Pgid]struct{}),
+		allocator: a,
+		pending:   make(map[common.Txid]*txPending),
+		allocs:    make(map[common.Pgid]common.Txid),
+		cache:     make(map[common.Pgid]struct{}),
 	}
 }
 
+func (t *shared) Init(ids common.Pgids) {
+	t.allocator.Init(ids)
+	t.reindex()
+}
+
+func (t *shared) Allocate(txid common.Txid, n int) common.Pgid {
+	return t.allocator.alloc(txid, n, &t.allocs, &t.cache)
+}
+
 func (t *shared) pendingPageIds() map[common.Txid]*txPending {
 	return t.pending
 }
@@ -316,3 +326,13 @@ func (t *shared) Write(p *common.Page) {
 		t.Copyall(ids[1:])
 	}
 }
+
+// functions to decouple array and hashmap
+
+func (t *shared) deleteFromCache(id common.Pgid) {
+	delete(t.cache, id)
+}
+
+func (t *shared) registerTransaction(pgid common.Pgid, txid common.Txid) {
+	t.allocs[pgid] = txid
+}