From 33b71a50af6bdb9dfcf0b79fa2b4ed378e5b679d Mon Sep 17 00:00:00 2001 From: Thomas Jungblut Date: Mon, 5 Aug 2024 10:49:05 +0200 Subject: [PATCH] Dedupe Reload/NoSyncReload, prefer empty instead of nil init Reload and NoSyncReload have duplicated code, this unifies both for later refactoring. This PR is split from #786, where the tests found differences on reloading and nil/empty initializations. Added some more clarifications in godocs for certain panic behavior and expected returns on the interface. Signed-off-by: Thomas Jungblut --- internal/freelist/freelist.go | 6 +++--- internal/freelist/shared.go | 24 +++--------------------- 2 files changed, 6 insertions(+), 24 deletions(-) diff --git a/internal/freelist/freelist.go b/internal/freelist/freelist.go index 3d77d8f94..2b819506b 100644 --- a/internal/freelist/freelist.go +++ b/internal/freelist/freelist.go @@ -11,7 +11,7 @@ type ReadWriter interface { // Write writes the freelist into the given page. Write(page *common.Page) - // EstimatedWritePageSize returns the size of the freelist after serialization in Write. + // EstimatedWritePageSize returns the size in bytes of the freelist after serialization in Write. // This should never underestimate the size. EstimatedWritePageSize() int } @@ -46,7 +46,7 @@ type Interface interface { ReleasePendingPages() // Free releases a page and its overflow for a given transaction id. - // If the page is already free then a panic will occur. + // If the page is already free or is one of the meta pages, then a panic will occur. Free(txId common.Txid, p *common.Page) // Freed returns whether a given page is in the free list. @@ -65,7 +65,7 @@ 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 returns the IDs of all free pages. Returns an empty slice if no free pages are available. freePageIds() common.Pgids // pendingPageIds returns all pending pages by transaction id. diff --git a/internal/freelist/shared.go b/internal/freelist/shared.go index 008483441..16a5b3286 100644 --- a/internal/freelist/shared.go +++ b/internal/freelist/shared.go @@ -208,25 +208,7 @@ func (t *shared) Copyall(dst []common.Pgid) { func (t *shared) Reload(p *common.Page) { t.Read(p) - - // Build a cache of only pending pages. - pcache := make(map[common.Pgid]bool) - for _, txp := range t.pending { - for _, pendingID := range txp.ids { - pcache[pendingID] = true - } - } - - // Check each page in the freelist and build a new available freelist - // with any pages not in the pending lists. - var a []common.Pgid - for _, id := range t.freePageIds() { - if !pcache[id] { - a = append(a, id) - } - } - - t.Init(a) + t.NoSyncReload(t.freePageIds()) } func (t *shared) NoSyncReload(pgIds common.Pgids) { @@ -240,7 +222,7 @@ func (t *shared) NoSyncReload(pgIds common.Pgids) { // Check each page in the freelist and build a new available freelist // with any pages not in the pending lists. - var a []common.Pgid + a := []common.Pgid{} for _, id := range pgIds { if !pcache[id] { a = append(a, id) @@ -274,7 +256,7 @@ func (t *shared) Read(p *common.Page) { // Copy the list of page ids from the freelist. if len(ids) == 0 { - t.Init(nil) + t.Init([]common.Pgid{}) } else { // copy the ids, so we don't modify on the freelist page directly idsCopy := make([]common.Pgid, len(ids))