From 7c1333850e17f3af381e7fab04f3dc91891c05ce Mon Sep 17 00:00:00 2001 From: Boris Kozlov Date: Fri, 21 Jul 2023 15:23:42 +0200 Subject: [PATCH 1/3] Adding an array of state lists --- .../herocache/backdata/heropool/linkedlist.go | 9 + .../herocache/backdata/heropool/pool.go | 234 +++++++----------- .../herocache/backdata/heropool/pool_test.go | 113 ++++----- 3 files changed, 156 insertions(+), 200 deletions(-) diff --git a/module/mempool/herocache/backdata/heropool/linkedlist.go b/module/mempool/herocache/backdata/heropool/linkedlist.go index 99823070d69..30414ed9de5 100644 --- a/module/mempool/herocache/backdata/heropool/linkedlist.go +++ b/module/mempool/herocache/backdata/heropool/linkedlist.go @@ -15,3 +15,12 @@ type state struct { tail EIndex size uint32 } + +// NewStates constructs an array of a doubly linked-lists. +func NewStates(numberOfStates int) []state { + result := make([]state, numberOfStates) + for i := 1; i < numberOfStates; i++ { + result[i] = state{head: InvalidIndex, tail: InvalidIndex, size: 0} + } + return result +} diff --git a/module/mempool/herocache/backdata/heropool/pool.go b/module/mempool/herocache/backdata/heropool/pool.go index 87e21abb33e..d8c4140ebc1 100644 --- a/module/mempool/herocache/backdata/heropool/pool.go +++ b/module/mempool/herocache/backdata/heropool/pool.go @@ -1,7 +1,6 @@ package heropool import ( - "fmt" "math" "github.com/rs/zerolog" @@ -18,21 +17,21 @@ const ( NoEjection = EjectionMode("no-ejection") ) +// StateIndex is a type of a state of a placeholder in a pool. +type StateIndex uint + +const numberOfStates = 2 +const ( // iota is reset to 0 + stateFree StateIndex = iota + stateUsed +) + // EIndex is data type representing an entity index in Pool. type EIndex uint32 // InvalidIndex is used when a link doesnt point anywhere, in other words it is an equivalent of a nil address. const InvalidIndex EIndex = math.MaxUint32 -// A type dedicated to describe possible states of placeholders for entities in the pool. -type StateType string - -// A placeholder in a free state can be used to store an entity. -const stateFree StateType = "free-state" - -// A placeholder in a used state stores currently an entity. -const stateUsed StateType = "used-state" - // poolEntity represents the data type that is maintained by type poolEntity struct { PoolEntity @@ -64,8 +63,7 @@ func (p PoolEntity) Entity() flow.Entity { type Pool struct { logger zerolog.Logger - free state // keeps track of free slots. - used state // keeps track of allocated slots to cachedEntities. + states []state // keeps track of a slot's state poolEntities []poolEntity ejectionMode EjectionMode } @@ -74,16 +72,8 @@ type Pool struct { // logger and a provided fixed size. func NewHeroPool(sizeLimit uint32, ejectionMode EjectionMode, logger zerolog.Logger) *Pool { l := &Pool{ - free: state{ - head: InvalidIndex, - tail: InvalidIndex, - size: 0, - }, - used: state{ - head: InvalidIndex, - tail: InvalidIndex, - size: 0, - }, + //constructor for states make them invalid + states: NewStates(numberOfStates), poolEntities: make([]poolEntity, sizeLimit), ejectionMode: ejectionMode, logger: logger, @@ -105,8 +95,16 @@ func (p *Pool) setDefaultNodeLinkValues() { // initFreeEntities initializes the free double linked-list with the indices of all cached entity poolEntities. func (p *Pool) initFreeEntities() { - for i := 0; i < len(p.poolEntities); i++ { - p.appendEntity(stateFree, EIndex(i)) + p.states[stateFree].head = 0 + p.states[stateFree].tail = 0 + p.poolEntities[p.states[stateFree].head].node.prev = InvalidIndex + p.poolEntities[p.states[stateFree].tail].node.next = InvalidIndex + p.states[stateFree].size = 1 + for i := 1; i < len(p.poolEntities); i++ { + p.connect(p.states[stateFree].tail, EIndex(i)) + p.states[stateFree].tail = EIndex(i) + p.poolEntities[p.states[stateFree].tail].node.next = InvalidIndex + p.states[stateFree].size++ } } @@ -124,7 +122,7 @@ func (p *Pool) Add(entityId flow.Identifier, entity flow.Entity, owner uint64) ( p.poolEntities[entityIndex].entity = entity p.poolEntities[entityIndex].id = entityId p.poolEntities[entityIndex].owner = owner - p.appendEntity(stateUsed, entityIndex) + p.switchState(stateFree, stateUsed, entityIndex) } return entityIndex, slotAvailable, ejectedEntity @@ -137,10 +135,10 @@ func (p *Pool) Get(entityIndex EIndex) (flow.Identifier, flow.Entity, uint64) { // All returns all stored entities in this pool. func (p Pool) All() []PoolEntity { - all := make([]PoolEntity, p.used.size) - next := p.used.head + all := make([]PoolEntity, p.states[stateUsed].size) + next := p.states[stateUsed].head - for i := uint32(0); i < p.used.size; i++ { + for i := uint32(0); i < p.states[stateUsed].size; i++ { e := p.poolEntities[next] all[i] = e.PoolEntity next = e.node.next @@ -149,13 +147,14 @@ func (p Pool) All() []PoolEntity { return all } -// Head returns the head of used items. Assuming no ejection happened and pool never goes beyond limit, Head returns +// Head returns the head of states[stateUsed] items. Assuming no ejection happened and pool never goes beyond limit, Head returns // the first inserted element. func (p Pool) Head() (flow.Entity, bool) { - if p.used.size == 0 { + + if p.states[stateUsed].size == 0 { return nil, false } - e := p.poolEntities[p.used.head] + e := p.poolEntities[p.states[stateUsed].head] return e.Entity(), true } @@ -169,12 +168,12 @@ func (p Pool) Head() (flow.Entity, bool) { func (p *Pool) sliceIndexForEntity() (i EIndex, hasAvailableSlot bool, ejectedEntity flow.Entity) { lruEject := func() (EIndex, bool, flow.Entity) { // LRU ejection - // the used head is the oldest entity, so we turn the used head to a free head here. + // the states[stateUsed] head is the oldest entity, so we turn the states[stateUsed] head to a states[stateFree] head here. invalidatedEntity := p.invalidateUsedHead() - return p.claimFreeHead(), true, invalidatedEntity + return p.states[stateFree].head, true, invalidatedEntity } - if p.free.size == 0 { + if p.states[stateFree].size == 0 { // the free list is empty, so we are out of space, and we need to eject. switch p.ejectionMode { case NoEjection: @@ -182,7 +181,7 @@ func (p *Pool) sliceIndexForEntity() (i EIndex, hasAvailableSlot bool, ejectedEn return InvalidIndex, false, nil case RandomEjection: // we only eject randomly when the pool is full and random ejection is on. - random, err := rand.Uint32n(p.used.size) + random, err := rand.Uint32n(p.states[stateUsed].size) if err != nil { p.logger.Fatal().Err(err). Msg("hero pool random ejection failed - falling back to LRU ejection") @@ -191,45 +190,44 @@ func (p *Pool) sliceIndexForEntity() (i EIndex, hasAvailableSlot bool, ejectedEn } randomIndex := EIndex(random) invalidatedEntity := p.invalidateEntityAtIndex(randomIndex) - return p.claimFreeHead(), true, invalidatedEntity + return p.states[stateFree].head, true, invalidatedEntity case LRUEjection: // LRU ejection return lruEject() } } - // claiming the head of free list as the slice index for the next entity to be added - return p.claimFreeHead(), true, nil + // returning the head of states[stateFree] list as the slice index for the next entity to be added + return p.states[stateFree].head, true, nil } // Size returns total number of entities that this list maintains. func (p Pool) Size() uint32 { - return p.used.size + return p.states[stateUsed].size } // getHeads returns entities corresponding to the used and free heads. -func (p *Pool) getHeads() (*poolEntity, *poolEntity) { +func (p Pool) getHeads() (*poolEntity, *poolEntity) { var usedHead, freeHead *poolEntity - if p.used.size != 0 { - usedHead = &p.poolEntities[p.used.head] + if p.states[stateUsed].size != 0 { + usedHead = &p.poolEntities[p.states[stateUsed].head] } - if p.free.size != 0 { - freeHead = &p.poolEntities[p.free.head] + if p.states[stateFree].size != 0 { + freeHead = &p.poolEntities[p.states[stateFree].head] } return usedHead, freeHead } -// getTails returns entities corresponding to the used and free tails. -func (p *Pool) getTails() (*poolEntity, *poolEntity) { +// getTails returns entities corresponding to the states[stateUsed] and states[stateFree] tails. +func (p Pool) getTails() (*poolEntity, *poolEntity) { var usedTail, freeTail *poolEntity - if p.used.size != 0 { - usedTail = &p.poolEntities[p.used.tail] + if p.states[stateUsed].size != 0 { + usedTail = &p.poolEntities[p.states[stateUsed].tail] } - - if p.free.size != 0 { - freeTail = &p.poolEntities[p.free.tail] + if p.states[stateFree].size != 0 { + freeTail = &p.poolEntities[p.states[stateFree].tail] } return usedTail, freeTail @@ -241,40 +239,28 @@ func (p *Pool) connect(prev EIndex, next EIndex) { p.poolEntities[next].node.prev = prev } -// invalidateUsedHead moves current used head forward by one node. It +// invalidateUsedHead moves current states[stateUsed] head forward by one node. It // also removes the entity the invalidated head is presenting and appends the -// node represented by the used head to the tail of the free list. +// node represented by the states[stateUsed] head to the tail of the states[stateFree] list. func (p *Pool) invalidateUsedHead() flow.Entity { - headSliceIndex := p.used.head + headSliceIndex := p.states[stateUsed].head return p.invalidateEntityAtIndex(headSliceIndex) } -// claimFreeHead moves the free head forward, and returns the slice index of the -// old free head to host a new entity. -func (p *Pool) claimFreeHead() EIndex { - oldFreeHeadIndex := p.free.head - p.removeEntity(stateFree, oldFreeHeadIndex) - return oldFreeHeadIndex -} - // Remove removes entity corresponding to given getSliceIndex from the list. func (p *Pool) Remove(sliceIndex EIndex) flow.Entity { return p.invalidateEntityAtIndex(sliceIndex) } // invalidateEntityAtIndex invalidates the given getSliceIndex in the linked list by -// removing its corresponding linked-list node from the used linked list, and appending -// it to the tail of the free list. It also removes the entity that the invalidated node is presenting. +// removing its corresponding linked-list node from the states[stateUsed] linked list, and appending +// it to the tail of the states[stateFree] list. It also removes the entity that the invalidated node is presenting. func (p *Pool) invalidateEntityAtIndex(sliceIndex EIndex) flow.Entity { - invalidatedEntity := p.poolEntities[sliceIndex].entity - if invalidatedEntity == nil { - panic(fmt.Sprintf("removing an entity from an empty slot with an index : %d", sliceIndex)) - } - p.removeEntity(stateUsed, sliceIndex) + poolEntity := p.poolEntities[sliceIndex] + invalidatedEntity := poolEntity.entity + p.switchState(stateUsed, stateFree, sliceIndex) p.poolEntities[sliceIndex].id = flow.ZeroID p.poolEntities[sliceIndex].entity = nil - p.appendEntity(stateFree, EIndex(sliceIndex)) - return invalidatedEntity } @@ -292,77 +278,47 @@ func (p *Pool) isInvalidated(sliceIndex EIndex) bool { return true } -// utility method that removes an entity from one of the states. -// NOTE: a removed entity has to be added to another state. -func (p *Pool) removeEntity(stateType StateType, entityIndex EIndex) { - var s *state = nil - switch stateType { - case stateFree: - s = &p.free - case stateUsed: - s = &p.used - default: - panic(fmt.Sprintf("unknown state type: %s", stateType)) - } - - if s.size == 0 { +// switches state of an entity. +func (p *Pool) switchState(stateFrom StateIndex, stateTo StateIndex, entityIndex EIndex) error { + // Remove from stateFrom list + if p.states[stateFrom].size == 0 { panic("Removing an entity from an empty list") - } - if s.size == 1 { - // here set to InvalidIndex - s.head = InvalidIndex - s.tail = InvalidIndex - s.size-- - p.poolEntities[entityIndex].node.next = InvalidIndex - p.poolEntities[entityIndex].node.prev = InvalidIndex - return - } - node := p.poolEntities[entityIndex].node - - if entityIndex != s.head && entityIndex != s.tail { - // links next and prev elements for non-head and non-tail element - p.connect(node.prev, node.next) - } - - if entityIndex == s.head { - // moves head forward - s.head = node.next - p.poolEntities[s.head].node.prev = InvalidIndex - } + } else if p.states[stateFrom].size == 1 { + p.states[stateFrom].head = InvalidIndex + p.states[stateFrom].tail = InvalidIndex + } else { + node := p.poolEntities[entityIndex].node + + if entityIndex != p.states[stateFrom].head && entityIndex != p.states[stateFrom].tail { + // links next and prev elements for non-head and non-tail element + p.connect(node.prev, node.next) + } - if entityIndex == s.tail { - // moves tail backwards - s.tail = node.prev - p.poolEntities[s.tail].node.next = InvalidIndex - } - s.size-- - p.poolEntities[entityIndex].node.next = InvalidIndex - p.poolEntities[entityIndex].node.prev = InvalidIndex -} + if entityIndex == p.states[stateFrom].head { + // moves head forward + p.states[stateFrom].head = node.next + p.poolEntities[p.states[stateFrom].head].node.prev = InvalidIndex + } -// appends an entity to the tail of the state or creates a first element. -// NOTE: entity should not be in any list before this method is applied -func (p *Pool) appendEntity(stateType StateType, entityIndex EIndex) { - var s *state = nil - switch stateType { - case stateFree: - s = &p.free - case stateUsed: - s = &p.used - default: - panic(fmt.Sprintf("unknown state type: %s", stateType)) + if entityIndex == p.states[stateFrom].tail { + // moves tail backwards + p.states[stateFrom].tail = node.prev + p.poolEntities[p.states[stateFrom].tail].node.next = InvalidIndex + } } - - if s.size == 0 { - s.head = entityIndex - s.tail = entityIndex - p.poolEntities[s.head].node.prev = InvalidIndex - p.poolEntities[s.tail].node.next = InvalidIndex - s.size = 1 - return + p.states[stateFrom].size-- + + // Add to stateTo list + if p.states[stateTo].size == 0 { + p.states[stateTo].head = entityIndex + p.states[stateTo].tail = entityIndex + p.poolEntities[p.states[stateTo].head].node.prev = InvalidIndex + p.poolEntities[p.states[stateTo].tail].node.next = InvalidIndex + } else { + p.connect(p.states[stateTo].tail, entityIndex) + p.states[stateTo].tail = entityIndex + p.poolEntities[p.states[stateTo].tail].node.next = InvalidIndex } - p.connect(s.tail, entityIndex) - s.size++ - s.tail = entityIndex - p.poolEntities[s.tail].node.next = InvalidIndex + p.states[stateTo].size++ + return nil } diff --git a/module/mempool/herocache/backdata/heropool/pool_test.go b/module/mempool/herocache/backdata/heropool/pool_test.go index 5cc19609753..07ca3c366dc 100644 --- a/module/mempool/herocache/backdata/heropool/pool_test.go +++ b/module/mempool/herocache/backdata/heropool/pool_test.go @@ -380,17 +380,17 @@ func testInvalidatingHead(t *testing.T, pool *Pool, entities []*unittest.MockEnt // size of list should be decremented after each invalidation. require.Equal(t, uint32(totalEntitiesStored-i-1), pool.Size()) // invalidated head should be appended to free entities - require.Equal(t, pool.free.tail, EIndex(i)) + require.Equal(t, pool.states[stateFree].tail, EIndex(i)) if freeListInitialSize != 0 { // number of entities is below limit, hence free list is not empty. // invalidating used head must not change the free head. - require.Equal(t, EIndex(totalEntitiesStored), pool.free.head) + require.Equal(t, EIndex(totalEntitiesStored), pool.states[stateFree].head) } else { // number of entities is greater than or equal to limit, hence free list is empty. // free head must be updated to the first invalidated head (index 0), // and must be kept there for entire test (as we invalidate head not tail). - require.Equal(t, EIndex(0), pool.free.head) + require.Equal(t, EIndex(0), pool.states[stateFree].head) } // except when the list is empty, head must be updated after invalidation, @@ -399,14 +399,14 @@ func testInvalidatingHead(t *testing.T, pool *Pool, entities []*unittest.MockEnt if i != totalEntitiesStored-1 { // used linked-list tailAccessibleFromHead(t, - pool.used.head, - pool.used.tail, + pool.states[stateUsed].head, + pool.states[stateUsed].tail, pool, pool.Size()) headAccessibleFromTail(t, - pool.used.head, - pool.used.tail, + pool.states[stateUsed].head, + pool.states[stateUsed].tail, pool, pool.Size()) @@ -414,14 +414,14 @@ func testInvalidatingHead(t *testing.T, pool *Pool, entities []*unittest.MockEnt // // after invalidating each item, size of free linked-list is incremented by one. tailAccessibleFromHead(t, - pool.free.head, - pool.free.tail, + pool.states[stateFree].head, + pool.states[stateFree].tail, pool, uint32(i+1+freeListInitialSize)) headAccessibleFromTail(t, - pool.free.head, - pool.free.tail, + pool.states[stateFree].head, + pool.states[stateFree].tail, pool, uint32(i+1+freeListInitialSize)) } @@ -435,21 +435,21 @@ func testInvalidatingHead(t *testing.T, pool *Pool, entities []*unittest.MockEnt // used tail should point to the last element in pool, since we are // invalidating head. require.Equal(t, entities[totalEntitiesStored-1].ID(), usedTail.id) - require.Equal(t, EIndex(totalEntitiesStored-1), pool.used.tail) + require.Equal(t, EIndex(totalEntitiesStored-1), pool.states[stateUsed].tail) // used head must point to the next element in the pool, // i.e., invalidating head moves it forward. require.Equal(t, entities[i+1].ID(), usedHead.id) - require.Equal(t, EIndex(i+1), pool.used.head) + require.Equal(t, EIndex(i+1), pool.states[stateUsed].head) } else { // pool is empty // used head and tail must be nil and their corresponding // pointer indices must be undefined. require.Nil(t, usedHead) require.Nil(t, usedTail) - require.True(t, pool.used.size == 0) - require.Equal(t, pool.used.tail, InvalidIndex) - require.Equal(t, pool.used.head, InvalidIndex) + require.True(t, pool.states[stateUsed].size == 0) + require.Equal(t, pool.states[stateUsed].tail, InvalidIndex) + require.Equal(t, pool.states[stateUsed].head, InvalidIndex) } checkEachEntityIsInFreeOrUsedState(t, pool) } @@ -461,25 +461,25 @@ func testInvalidatingTail(t *testing.T, pool *Pool, entities []*unittest.MockEnt offset := len(pool.poolEntities) - size for i := 0; i < size; i++ { // invalidates tail index - tailIndex := pool.used.tail + tailIndex := pool.states[stateUsed].tail require.Equal(t, EIndex(size-1-i), tailIndex) pool.invalidateEntityAtIndex(tailIndex) // old head index must be invalidated require.True(t, pool.isInvalidated(tailIndex)) // unclaimed head should be appended to free entities - require.Equal(t, pool.free.tail, tailIndex) + require.Equal(t, pool.states[stateFree].tail, tailIndex) if offset != 0 { // number of entities is below limit // free must head keeps pointing to first empty index after // adding all entities. - require.Equal(t, EIndex(size), pool.free.head) + require.Equal(t, EIndex(size), pool.states[stateFree].head) } else { // number of entities is greater than or equal to limit // free head must be updated to last element in the pool (size - 1), // and must be kept there for entire test (as we invalidate tail not head). - require.Equal(t, EIndex(size-1), pool.free.head) + require.Equal(t, EIndex(size-1), pool.states[stateFree].head) } // size of pool should be shrunk after each invalidation. @@ -492,27 +492,27 @@ func testInvalidatingTail(t *testing.T, pool *Pool, entities []*unittest.MockEnt // used linked-list tailAccessibleFromHead(t, - pool.used.head, - pool.used.tail, + pool.states[stateUsed].head, + pool.states[stateUsed].tail, pool, pool.Size()) headAccessibleFromTail(t, - pool.used.head, - pool.used.tail, + pool.states[stateUsed].head, + pool.states[stateUsed].tail, pool, pool.Size()) // free linked-list tailAccessibleFromHead(t, - pool.free.head, - pool.free.tail, + pool.states[stateFree].head, + pool.states[stateFree].tail, pool, uint32(i+1+offset)) headAccessibleFromTail(t, - pool.free.head, - pool.free.tail, + pool.states[stateFree].head, + pool.states[stateFree].tail, pool, uint32(i+1+offset)) } @@ -524,20 +524,20 @@ func testInvalidatingTail(t *testing.T, pool *Pool, entities []*unittest.MockEnt // // used tail should move backward after each invalidation require.Equal(t, entities[size-i-2].ID(), usedTail.id) - require.Equal(t, EIndex(size-i-2), pool.used.tail) + require.Equal(t, EIndex(size-i-2), pool.states[stateUsed].tail) // used head must point to the first element in the pool, require.Equal(t, entities[0].ID(), usedHead.id) - require.Equal(t, EIndex(0), pool.used.head) + require.Equal(t, EIndex(0), pool.states[stateUsed].head) } else { // pool is empty // used head and tail must be nil and their corresponding // pointer indices must be undefined. require.Nil(t, usedHead) require.Nil(t, usedTail) - require.True(t, pool.used.size == 0) - require.Equal(t, pool.used.head, InvalidIndex) - require.Equal(t, pool.used.tail, InvalidIndex) + require.True(t, pool.states[stateUsed].size == 0) + require.Equal(t, pool.states[stateUsed].head, InvalidIndex) + require.Equal(t, pool.states[stateUsed].tail, InvalidIndex) } checkEachEntityIsInFreeOrUsedState(t, pool) } @@ -546,14 +546,14 @@ func testInvalidatingTail(t *testing.T, pool *Pool, entities []*unittest.MockEnt // testInitialization evaluates the state of an initialized pool before adding any element to it. func testInitialization(t *testing.T, pool *Pool, _ []*unittest.MockEntity) { // "used" linked-list must have a zero size, since we have no elements in the list. - require.True(t, pool.used.size == 0) - require.Equal(t, pool.used.head, InvalidIndex) - require.Equal(t, pool.used.tail, InvalidIndex) + require.True(t, pool.states[stateUsed].size == 0) + require.Equal(t, pool.states[stateUsed].head, InvalidIndex) + require.Equal(t, pool.states[stateUsed].tail, InvalidIndex) for i := 0; i < len(pool.poolEntities); i++ { if i == 0 { // head of "free" linked-list should point to InvalidIndex of entities slice. - require.Equal(t, EIndex(i), pool.free.head) + require.Equal(t, EIndex(i), pool.states[stateFree].head) // previous element of head must be undefined (linked-list head feature). require.Equal(t, pool.poolEntities[i].node.prev, InvalidIndex) } @@ -570,7 +570,7 @@ func testInitialization(t *testing.T, pool *Pool, _ []*unittest.MockEntity) { if i == len(pool.poolEntities)-1 { // tail of "free" linked-list should point to the last index in entities slice. - require.Equal(t, EIndex(i), pool.free.tail) + require.Equal(t, EIndex(i), pool.states[stateFree].tail) // next element of tail must be undefined. require.Equal(t, pool.poolEntities[i].node.next, InvalidIndex) } @@ -690,7 +690,7 @@ func testAddingEntities(t *testing.T, pool *Pool, entitiesToBeAdded []*unittest. if i < len(pool.poolEntities)-1 { // as long as we are below limit, after adding i element, free head // should move to i+1 element. - require.Equal(t, EIndex(i+1), pool.free.head) + require.Equal(t, EIndex(i+1), pool.states[stateFree].head) // head must be healthy and point back to undefined. require.Equal(t, freeHead.node.prev, InvalidIndex) } else { @@ -706,7 +706,7 @@ func testAddingEntities(t *testing.T, pool *Pool, entitiesToBeAdded []*unittest. // must keep pointing to last index of the array-based linked-list. In other // words, adding element must not change free tail (since only free head is // updated). - require.Equal(t, EIndex(len(pool.poolEntities)-1), pool.free.tail) + require.Equal(t, EIndex(len(pool.poolEntities)-1), pool.states[stateFree].tail) // head tail be healthy and point next to undefined. require.Equal(t, freeTail.node.next, InvalidIndex) } else { @@ -726,13 +726,13 @@ func testAddingEntities(t *testing.T, pool *Pool, entitiesToBeAdded []*unittest. usedTraverseStep = uint32(len(pool.poolEntities)) } tailAccessibleFromHead(t, - pool.used.head, - pool.used.tail, + pool.states[stateUsed].head, + pool.states[stateUsed].tail, pool, usedTraverseStep) headAccessibleFromTail(t, - pool.used.head, - pool.used.tail, + pool.states[stateUsed].head, + pool.states[stateUsed].tail, pool, usedTraverseStep) @@ -750,13 +750,13 @@ func testAddingEntities(t *testing.T, pool *Pool, entitiesToBeAdded []*unittest. freeTraverseStep = uint32(0) } tailAccessibleFromHead(t, - pool.free.head, - pool.free.tail, + pool.states[stateFree].head, + pool.states[stateFree].tail, pool, freeTraverseStep) headAccessibleFromTail(t, - pool.free.head, - pool.free.tail, + pool.states[stateFree].head, + pool.states[stateFree].tail, pool, freeTraverseStep) @@ -804,7 +804,7 @@ func withTestScenario(t *testing.T, pool := NewHeroPool(limit, ejectionMode, unittest.Logger()) // head on underlying linked-list value should be uninitialized - require.True(t, pool.used.size == 0) + require.True(t, pool.states[stateUsed].size == 0) require.Equal(t, pool.Size(), uint32(0)) entities := unittest.EntityListFixture(uint(entityCount)) @@ -857,7 +857,7 @@ func headAccessibleFromTail(t *testing.T, headSliceIndex EIndex, tailSliceIndex func checkEachEntityIsInFreeOrUsedState(t *testing.T, pool *Pool) { pool_capacity := len(pool.poolEntities) // check size - require.Equal(t, int(pool.free.size+pool.used.size), pool_capacity, "Pool capacity is not equal to the sum of used and free sizes") + require.Equal(t, int(pool.states[stateFree].size+pool.states[stateUsed].size), pool_capacity, "Pool capacity is not equal to the sum of used and free sizes") // check elelments nodesInFree := discoverEntitiesBelongingToStateList(t, pool, stateFree) nodesInUsed := discoverEntitiesBelongingToStateList(t, pool, stateUsed) @@ -868,18 +868,9 @@ func checkEachEntityIsInFreeOrUsedState(t *testing.T, pool *Pool) { } // discoverEntitiesBelongingToStateList discovers all entities in the pool that belong to the given list. -func discoverEntitiesBelongingToStateList(t *testing.T, pool *Pool, stateType StateType) []bool { - var s *state = nil - switch stateType { - case stateFree: - s = &pool.free - case stateUsed: - s = &pool.used - default: - panic(fmt.Sprintf("unknown state type: %s", stateType)) - } +func discoverEntitiesBelongingToStateList(t *testing.T, pool *Pool, stateType StateIndex) []bool { result := make([]bool, len(pool.poolEntities)) - for node_index := s.head; node_index != InvalidIndex; { + for node_index := pool.states[stateType].head; node_index != InvalidIndex; { require.False(t, result[node_index], "A node is present two times in the same state list") result[node_index] = true node_index = pool.poolEntities[node_index].node.next From 3b5cc0ffad5a27a47ac43b4e3f1360dd894d1c83 Mon Sep 17 00:00:00 2001 From: Boris Kozlov Date: Fri, 4 Aug 2023 16:09:12 +0200 Subject: [PATCH 2/3] Improved comments --- module/mempool/herocache/backdata/heropool/pool.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/module/mempool/herocache/backdata/heropool/pool.go b/module/mempool/herocache/backdata/heropool/pool.go index d8c4140ebc1..23e49cea9d7 100644 --- a/module/mempool/herocache/backdata/heropool/pool.go +++ b/module/mempool/herocache/backdata/heropool/pool.go @@ -1,6 +1,7 @@ package heropool import ( + "fmt" "math" "github.com/rs/zerolog" @@ -72,7 +73,7 @@ type Pool struct { // logger and a provided fixed size. func NewHeroPool(sizeLimit uint32, ejectionMode EjectionMode, logger zerolog.Logger) *Pool { l := &Pool{ - //constructor for states make them invalid + //construcs states initialized to InvalidIndexes states: NewStates(numberOfStates), poolEntities: make([]poolEntity, sizeLimit), ejectionMode: ejectionMode, @@ -150,7 +151,6 @@ func (p Pool) All() []PoolEntity { // Head returns the head of states[stateUsed] items. Assuming no ejection happened and pool never goes beyond limit, Head returns // the first inserted element. func (p Pool) Head() (flow.Entity, bool) { - if p.states[stateUsed].size == 0 { return nil, false } @@ -256,8 +256,10 @@ func (p *Pool) Remove(sliceIndex EIndex) flow.Entity { // removing its corresponding linked-list node from the states[stateUsed] linked list, and appending // it to the tail of the states[stateFree] list. It also removes the entity that the invalidated node is presenting. func (p *Pool) invalidateEntityAtIndex(sliceIndex EIndex) flow.Entity { - poolEntity := p.poolEntities[sliceIndex] - invalidatedEntity := poolEntity.entity + invalidatedEntity := p.poolEntities[sliceIndex].entity + if invalidatedEntity == nil { + panic(fmt.Sprintf("removing an entity from an empty slot with an index : %d", sliceIndex)) + } p.switchState(stateUsed, stateFree, sliceIndex) p.poolEntities[sliceIndex].id = flow.ZeroID p.poolEntities[sliceIndex].entity = nil From 21129fe50c0e105b8892f7542ea6843289004173 Mon Sep 17 00:00:00 2001 From: Boris Kozlov Date: Fri, 25 Aug 2023 13:29:47 +0200 Subject: [PATCH 3/3] Applied minor comments --- .../herocache/backdata/heropool/pool.go | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/module/mempool/herocache/backdata/heropool/pool.go b/module/mempool/herocache/backdata/heropool/pool.go index 23e49cea9d7..54191cba592 100644 --- a/module/mempool/herocache/backdata/heropool/pool.go +++ b/module/mempool/herocache/backdata/heropool/pool.go @@ -148,7 +148,7 @@ func (p Pool) All() []PoolEntity { return all } -// Head returns the head of states[stateUsed] items. Assuming no ejection happened and pool never goes beyond limit, Head returns +// Head returns the head of used items. Assuming no ejection happened and pool never goes beyond limit, Head returns // the first inserted element. func (p Pool) Head() (flow.Entity, bool) { if p.states[stateUsed].size == 0 { @@ -159,6 +159,9 @@ func (p Pool) Head() (flow.Entity, bool) { } // sliceIndexForEntity returns a slice index which hosts the next entity to be added to the list. +// This index is invalid if there are no available slots or ejection could not be performed. +// If the valid index is returned then it is guaranteed that it corresponds to a free list head. +// Thus when filled with a new entity a switchState must be applied. // // The first boolean return value (hasAvailableSlot) says whether pool has an available slot. // Pool goes out of available slots if it is full and no ejection is set. @@ -168,7 +171,7 @@ func (p Pool) Head() (flow.Entity, bool) { func (p *Pool) sliceIndexForEntity() (i EIndex, hasAvailableSlot bool, ejectedEntity flow.Entity) { lruEject := func() (EIndex, bool, flow.Entity) { // LRU ejection - // the states[stateUsed] head is the oldest entity, so we turn the states[stateUsed] head to a states[stateFree] head here. + // the used head is the oldest entity, so we turn the used head to a free head here. invalidatedEntity := p.invalidateUsedHead() return p.states[stateFree].head, true, invalidatedEntity } @@ -197,7 +200,7 @@ func (p *Pool) sliceIndexForEntity() (i EIndex, hasAvailableSlot bool, ejectedEn } } - // returning the head of states[stateFree] list as the slice index for the next entity to be added + // returning the head of free list as the slice index for the next entity to be added return p.states[stateFree].head, true, nil } @@ -220,7 +223,7 @@ func (p Pool) getHeads() (*poolEntity, *poolEntity) { return usedHead, freeHead } -// getTails returns entities corresponding to the states[stateUsed] and states[stateFree] tails. +// getTails returns entities corresponding to the used and free tails. func (p Pool) getTails() (*poolEntity, *poolEntity) { var usedTail, freeTail *poolEntity if p.states[stateUsed].size != 0 { @@ -239,9 +242,9 @@ func (p *Pool) connect(prev EIndex, next EIndex) { p.poolEntities[next].node.prev = prev } -// invalidateUsedHead moves current states[stateUsed] head forward by one node. It +// invalidateUsedHead moves current used head forward by one node. It // also removes the entity the invalidated head is presenting and appends the -// node represented by the states[stateUsed] head to the tail of the states[stateFree] list. +// node represented by the used head to the tail of the free list. func (p *Pool) invalidateUsedHead() flow.Entity { headSliceIndex := p.states[stateUsed].head return p.invalidateEntityAtIndex(headSliceIndex) @@ -253,8 +256,8 @@ func (p *Pool) Remove(sliceIndex EIndex) flow.Entity { } // invalidateEntityAtIndex invalidates the given getSliceIndex in the linked list by -// removing its corresponding linked-list node from the states[stateUsed] linked list, and appending -// it to the tail of the states[stateFree] list. It also removes the entity that the invalidated node is presenting. +// removing its corresponding linked-list node from the used linked list, and appending +// it to the tail of the free list. It also removes the entity that the invalidated node is presenting. func (p *Pool) invalidateEntityAtIndex(sliceIndex EIndex) flow.Entity { invalidatedEntity := p.poolEntities[sliceIndex].entity if invalidatedEntity == nil { @@ -281,7 +284,7 @@ func (p *Pool) isInvalidated(sliceIndex EIndex) bool { } // switches state of an entity. -func (p *Pool) switchState(stateFrom StateIndex, stateTo StateIndex, entityIndex EIndex) error { +func (p *Pool) switchState(stateFrom StateIndex, stateTo StateIndex, entityIndex EIndex) { // Remove from stateFrom list if p.states[stateFrom].size == 0 { panic("Removing an entity from an empty list") @@ -322,5 +325,4 @@ func (p *Pool) switchState(stateFrom StateIndex, stateTo StateIndex, entityIndex p.poolEntities[p.states[stateTo].tail].node.next = InvalidIndex } p.states[stateTo].size++ - return nil }