From 9c62d7b2d112baae8af4745f6a66dc44196ed1b0 Mon Sep 17 00:00:00 2001 From: Gyuho Lee Date: Mon, 2 Apr 2018 16:31:58 -0700 Subject: [PATCH] leaes: remove unnecessary O(log N) operation when nothing is expiry Since heap is already sorted, we can just check first element to see if anything is expiry, rather than popping and pushing it back. If nothing is expiry, pop operation is unnecessary. Signed-off-by: Gyuho Lee --- lease/lease_queue_test.go | 4 ++++ lease/lessor.go | 8 ++++---- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/lease/lease_queue_test.go b/lease/lease_queue_test.go index 72346a1b8..201911fd0 100644 --- a/lease/lease_queue_test.go +++ b/lease/lease_queue_test.go @@ -52,4 +52,8 @@ func TestLeaseQueue(t *testing.T) { if more { t.Fatal("expect no more expiry lease") } + + if le.leaseHeap.Len() != 49 { + t.Fatalf("expected lease heap pop, got %d", le.leaseHeap.Len()) + } } diff --git a/lease/lessor.go b/lease/lessor.go index 21569394c..90c568f9d 100644 --- a/lease/lessor.go +++ b/lease/lessor.go @@ -522,30 +522,30 @@ func (le *lessor) runLoop() { } // expireExists returns true if expiry items exist. -// It pops "first" expired item from heap. -// If it's not expired yet, add it back. +// It pops only when expiry item exists. // "next" is true, to indicate that it may exist in next attempt. func (le *lessor) expireExists() (l *Lease, ok bool, next bool) { if le.leaseHeap.Len() == 0 { return nil, false, false } - item := heap.Pop(&le.leaseHeap).(*LeaseWithTime) // O(log N) + item := le.leaseHeap[0] l = le.leaseMap[item.id] if l == nil { // lease has expired or been revoked // no need to revoke (nothing is expiry) + heap.Pop(&le.leaseHeap) // O(log N) return nil, false, true } if time.Now().UnixNano() < item.expiration { // Candidate expirations are caught up, reinsert this item // and no need to revoke (nothing is expiry) - heap.Push(&le.leaseHeap, item) // O(log N) return l, false, false } // if the lease is actually expired, add to the removal list. If it is not expired, we can ignore it because another entry will have been inserted into the heap + heap.Pop(&le.leaseHeap) // O(log N) return l, true, false }