From 38176e188ec7d8a834e7924fe72a7e91146a6c89 Mon Sep 17 00:00:00 2001 From: kanishka Date: Sun, 18 Jun 2023 16:06:24 +0530 Subject: [PATCH] use types --- dot/parachain/dispute/queue_test.go | 27 +++++----- dot/parachain/dispute/queues.go | 77 ++++++++++++++--------------- 2 files changed, 50 insertions(+), 54 deletions(-) diff --git a/dot/parachain/dispute/queue_test.go b/dot/parachain/dispute/queue_test.go index 983818e061..41229718fd 100644 --- a/dot/parachain/dispute/queue_test.go +++ b/dot/parachain/dispute/queue_test.go @@ -2,6 +2,7 @@ package dispute import ( "encoding/binary" + "github.com/ChainSafe/gossamer/lib/parachain" "sync" "testing" @@ -19,11 +20,11 @@ func newTestQueue(size int) Queue { } } -func newComparator(blockNumber, order uint32) *CandidateComparator { +func newComparator(blockNumber, order uint32) CandidateComparator { candidateHash := make([]byte, 4) binary.LittleEndian.PutUint32(candidateHash, order) - return &CandidateComparator{ + return CandidateComparator{ relayParentBlockNumber: &blockNumber, candidateHash: common.NewHash(candidateHash), } @@ -32,7 +33,7 @@ func newComparator(blockNumber, order uint32) *CandidateComparator { func dummyParticipationRequest() *ParticipationRequest { return &ParticipationRequest{ candidateHash: [32]byte{}, - candidateReceipt: []byte{1, 2, 3}, + candidateReceipt: parachain.CandidateReceipt{}, session: 1, } } @@ -42,7 +43,7 @@ type tests struct { // operation one of "queue", "dequeue", "prioritise", "pop_priority", "pop_best_effort", // "len_priority", "len_best_effort" operation string - comparator *CandidateComparator + comparator CandidateComparator request *ParticipationRequest priority ParticipationPriority expected any @@ -65,7 +66,7 @@ func runTests(t *testing.T, tests []tests, queue Queue) { item := queue.Dequeue() require.Equal(t, tt.expected, item) case "prioritise": - err := queue.PrioritiseIfPresent(*tt.comparator) + err := queue.PrioritiseIfPresent(tt.comparator) if tt.mustError { require.Error(t, err) return @@ -125,7 +126,7 @@ func TestQueue_CompareRelayParentBlock(t *testing.T) { { name: "dequeue", operation: "dequeue", - expected: &queueItem{ + expected: &participationItem{ comparator: newComparator(1, 1), request: dummyParticipationRequest(), }, @@ -158,7 +159,7 @@ func TestQueue_CompareCandidateHash(t *testing.T) { { name: "dequeue", operation: "dequeue", - expected: &queueItem{ + expected: &participationItem{ comparator: newComparator(1, 1), request: dummyParticipationRequest(), }, @@ -231,7 +232,7 @@ func TestQueue_EndToEnd(t *testing.T) { { name: "pop priority", operation: "pop_priority", - expected: &queueItem{ + expected: &participationItem{ comparator: newComparator(1, 1), request: dummyParticipationRequest(), }, @@ -239,7 +240,7 @@ func TestQueue_EndToEnd(t *testing.T) { { name: "pop best effort", operation: "pop_best_effort", - expected: &queueItem{ + expected: &participationItem{ comparator: newComparator(1, 2), request: dummyParticipationRequest(), }, @@ -267,7 +268,7 @@ func TestQueue_EndToEnd(t *testing.T) { { name: "dequeue", operation: "dequeue", - expected: &queueItem{ + expected: &participationItem{ comparator: newComparator(2, 1), request: dummyParticipationRequest(), }, @@ -275,7 +276,7 @@ func TestQueue_EndToEnd(t *testing.T) { { name: "dequeue", operation: "dequeue", - expected: &queueItem{ + expected: &participationItem{ comparator: newComparator(2, 2), request: dummyParticipationRequest(), }, @@ -438,7 +439,7 @@ func TestQueueConcurrency_Prioritise(t *testing.T) { block := uint32(i) go func() { defer wg.Done() - err := q.PrioritiseIfPresent(*newComparator(block, 1)) + err := q.PrioritiseIfPresent(newComparator(block, 1)) require.NoError(t, err) }() } @@ -582,7 +583,7 @@ func BenchmarkQueue_PrioritiseIfPresent(b *testing.B) { b.ResetTimer() for i := 0; i < priorityQueueSize; i++ { - err := q.PrioritiseIfPresent(*newComparator(uint32(i), 1)) + err := q.PrioritiseIfPresent(newComparator(uint32(i), 1)) require.NoError(b, err) } } diff --git a/dot/parachain/dispute/queues.go b/dot/parachain/dispute/queues.go index 91dd0b3fd9..fdab029fbf 100644 --- a/dot/parachain/dispute/queues.go +++ b/dot/parachain/dispute/queues.go @@ -2,6 +2,8 @@ package dispute import ( "bytes" + "github.com/ChainSafe/gossamer/lib/common" + "github.com/ChainSafe/gossamer/lib/parachain" "sync" "github.com/google/btree" @@ -23,37 +25,27 @@ import ( // CandidateComparator comparator for ordering of disputes for candidate. type CandidateComparator struct { relayParentBlockNumber *uint32 - candidateHash [32]byte + candidateHash common.Hash } // ParticipationRequest a dispute participation request type ParticipationRequest struct { - candidateHash [32]byte - candidateReceipt []byte //TODO: use the actual type - session uint + candidateHash common.Hash + candidateReceipt parachain.CandidateReceipt + session parachain.SessionIndex //TODO: requestTimer for metrics } -// queueItem implements btree.Item -// TODO: the rust implementation uses BTreeMap. We call `ReplaceOrInsert` in Queue(). -// Doesn't make sense to use map[] here for now. Need to explore btree further to see if we can improve performance by -// using a map[]. -type queueItem struct { - comparator *CandidateComparator +// participationItem implements btree.Item +type participationItem struct { + comparator CandidateComparator request *ParticipationRequest } -func newQueueItem(comparator *CandidateComparator, request *ParticipationRequest) *queueItem { - return &queueItem{ - comparator: comparator, - request: request, - } -} - // Less returns true if the current item is less than the other item // it uses the CandidateComparator to determine the order -func (q queueItem) Less(than btree.Item) bool { - other := than.(*queueItem) +func (q participationItem) Less(than btree.Item) bool { + other := than.(*participationItem) if q.comparator.relayParentBlockNumber == nil && other.comparator.relayParentBlockNumber == nil { return bytes.Compare(q.comparator.candidateHash[:], other.comparator.candidateHash[:]) < 0 @@ -74,6 +66,13 @@ func (q queueItem) Less(than btree.Item) bool { return *q.comparator.relayParentBlockNumber < *other.comparator.relayParentBlockNumber } +func newParticipationItem(comparator CandidateComparator, request *ParticipationRequest) *participationItem { + return &participationItem{ + comparator: comparator, + request: request, + } +} + // ParticipationPriority the priority of a participation request type ParticipationPriority int @@ -84,6 +83,7 @@ const ( ParticipationPriorityHigh ) +// IsPriority returns true if the priority is high func (p ParticipationPriority) IsPriority() bool { return p == ParticipationPriorityHigh } @@ -98,19 +98,19 @@ var ( // Queue the dispute participation queue type Queue interface { // Queue adds a new participation request to the queue - Queue(comparator *CandidateComparator, request *ParticipationRequest, priority ParticipationPriority) error + Queue(comparator CandidateComparator, request *ParticipationRequest, priority ParticipationPriority) error // Dequeue gets the next best request for dispute participation if any. - Dequeue() *queueItem + Dequeue() *participationItem // PrioritiseIfPresent moves a participation request from the best effort queue to the priority queue PrioritiseIfPresent(comparator CandidateComparator) error // PopBestEffort removes the next participation request from the best effort queue - PopBestEffort() *queueItem + PopBestEffort() *participationItem // PopPriority removes the next participation request from the priority queue - PopPriority() *queueItem + PopPriority() *participationItem // Len returns the number of items in the specified queue Len(queueType ParticipationPriority) int @@ -118,8 +118,8 @@ type Queue interface { // queue implements Queue // It uses two btree's to store the requests. One for best effort and one for priority. -// The queues store queueItem's. -// The btree is ordered by the CandidateComparator of queueItem. +// The queues store participationItem's. +// The btree is ordered by the CandidateComparator of participationItem. type queue struct { bestEffort *btree.BTree priority *btree.BTree @@ -150,7 +150,7 @@ func NewQueue() Queue { } func (q *queue) Queue( - comparator *CandidateComparator, + comparator CandidateComparator, request *ParticipationRequest, priority ParticipationPriority, ) error { @@ -160,7 +160,7 @@ func (q *queue) Queue( } q.priorityLock.Lock() - q.priority.ReplaceOrInsert(newQueueItem(comparator, request)) + q.priority.ReplaceOrInsert(newParticipationItem(comparator, request)) q.priorityLock.Unlock() } else { if q.bestEffort.Len() >= q.bestEffortMaxSize { @@ -168,14 +168,14 @@ func (q *queue) Queue( } q.bestEffortLock.Lock() - q.bestEffort.ReplaceOrInsert(newQueueItem(comparator, request)) + q.bestEffort.ReplaceOrInsert(newParticipationItem(comparator, request)) q.bestEffortLock.Unlock() } return nil } -func (q *queue) Dequeue() *queueItem { +func (q *queue) Dequeue() *participationItem { if item := q.PopPriority(); item != nil { return item } @@ -190,7 +190,7 @@ func (q *queue) PrioritiseIfPresent(comparator CandidateComparator) error { q.bestEffortLock.Lock() // We remove the item from the best effort queue and add it to the priority queue if it exists - if item := q.bestEffort.Delete(newQueueItem(&comparator, nil)); item != nil { + if item := q.bestEffort.Delete(newParticipationItem(comparator, nil)); item != nil { q.priorityLock.Lock() q.priority.ReplaceOrInsert(item) q.priorityLock.Unlock() @@ -200,21 +200,21 @@ func (q *queue) PrioritiseIfPresent(comparator CandidateComparator) error { return nil } -func (q *queue) PopBestEffort() *queueItem { +func (q *queue) PopBestEffort() *participationItem { q.bestEffortLock.Lock() defer q.bestEffortLock.Unlock() - if item := pop(q.bestEffort); item != nil { - return item.(*queueItem) + if item := q.bestEffort.DeleteMin(); item != nil { + return item.(*participationItem) } return nil } -func (q *queue) PopPriority() *queueItem { +func (q *queue) PopPriority() *participationItem { q.priorityLock.Lock() defer q.priorityLock.Unlock() - if item := pop(q.priority); item != nil { - return item.(*queueItem) + if item := q.priority.DeleteMin(); item != nil { + return item.(*participationItem) } return nil @@ -227,8 +227,3 @@ func (q *queue) Len(queueType ParticipationPriority) int { return q.bestEffort.Len() } - -func pop(target *btree.BTree) btree.Item { - // DeleteMin returns the first item in the tree if any - return target.DeleteMin() -}