dnrops
/
swift-nio
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Reduce refcounting in Heap by removing configurability (#1614) 

Motivation:

Right now Heap.heapify incurs quite a lot of ARC traffic when it is used
in SelectableEventLoop. Specifically, each of the invocations of
self.comparator in heapify incurs three refcount operations. These three
are all the result of the fact that the comparator is an arbitrary
function, but all three are guaranteed to be unnecessary.

The first refcount is for the closure context for self.comparator. Of
course, self.comparator has only two possible values, neither of which
needs a closure context, but the compiler doesn't know that and the type
system doesn't encode it.

The other two refcounts are for the ScheduledTask objects themselves.
Again, the compiler doesn't know what the comparator function will do,
so it must defensively ensure that a retain/release is inserted for the
arguments of that function.

If we happen to hit both branches of the code, we'll execute 6 retains
and 6 releases, and even in fairly average cases we'll execute 3 of
each.

All of this is unnecessary: the only runtime function ever used here is
a trivial comparison operation. We know what it is at compile time, we
just have to ask. While it was admirable to have the configurability of
the priority queue in place, after 3 years I think we have a pretty good
feeling that we're not going to need the ability to have a max-heap
instead of a min-heap.

We can always get this back if we need it.

Modifications:

- Removed the configurability of Heap.
- Removed HeapType.
- Removed arguments from PriorityQueue
- Removed all tests testing now-removed max-heap functionality.

Result:

Less ARC traffic in the core task scheduling loop.
This commit is contained in:
Cory Benfield 2020-08-10 13:12:43 +01:00 committed by GitHub
parent 08706eb13e
commit 831cf89d54
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 57 additions and 133 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
Sources/NIO/Embedded.swift
View File

@ -2,7 +2,7 @@
//
// This source file is part of the SwiftNIO open source project
//
// Copyright (c) 2017-2018 Apple Inc. and the SwiftNIO project authors
// Copyright (c) 2017-2020 Apple Inc. and the SwiftNIO project authors
// Licensed under Apache License v2.0
//
// See LICENSE.txt for license information
@ -61,7 +61,7 @@ public final class EmbeddedEventLoop: EventLoop {
/// The current "time" for this event loop. This is an amount in nanoseconds.
/* private but tests */ internal var _now: NIODeadline = .uptimeNanoseconds(0)
private var scheduledTasks = PriorityQueue<EmbeddedScheduledTask>(ascending: true)
private var scheduledTasks = PriorityQueue<EmbeddedScheduledTask>()
// The number of the next task to be created. We track the order so that when we execute tasks
// scheduled at the same time, we may do so in the order in which they were submitted for

32
Sources/NIO/Heap.swift
View File

@ -2,7 +2,7 @@
//
// This source file is part of the SwiftNIO open source project
//
// Copyright (c) 2017-2018 Apple Inc. and the SwiftNIO project authors
// Copyright (c) 2017-2020 Apple Inc. and the SwiftNIO project authors
// Licensed under Apache License v2.0
//
// See LICENSE.txt for license information
@ -12,34 +12,17 @@
//
//===----------------------------------------------------------------------===//
@usableFromInline
internal enum HeapType {
case maxHeap
case minHeap
@inlinable
internal func comparator<T: Comparable>(type: T.Type) -> (T, T) -> Bool {
switch self {
case .maxHeap:
return (>)
case .minHeap:
return (<)
}
}
}
@usableFromInline
internal struct Heap<Element: Comparable> {
@usableFromInline
internal let type: HeapType
@usableFromInline
internal private(set) var storage: ContiguousArray<Element> = []
internal let comparator: (Element, Element) -> Bool
@usableFromInline
internal init(type: HeapType) {
self.comparator = type.comparator(type: Element.self)
self.type = type
internal init() {}
internal func comparator(_ lhs: Element, _ rhs: Element) -> Bool {
// This heap is always a min-heap.
return lhs < rhs
}
// named `PARENT` in CLRS
@ -126,10 +109,9 @@ internal struct Heap<Element: Comparable> {
return nil
}
let element = self.storage[index]
let comparator = self.comparator
if self.storage.count == 1 || self.storage[index] == self.storage[self.storage.count - 1] {
self.storage.removeLast()
} else if !comparator(self.storage[index], self.storage[self.storage.count - 1]) {
} else if !self.comparator(self.storage[index], self.storage[self.storage.count - 1]) {
self.heapRootify(index: index, key: self.storage[self.storage.count - 1])
self.storage.removeLast()
} else {

8
Sources/NIO/PriorityQueue.swift
View File

@ -2,7 +2,7 @@
//
// This source file is part of the SwiftNIO open source project
//
// Copyright (c) 2017-2018 Apple Inc. and the SwiftNIO project authors
// Copyright (c) 2017-2020 Apple Inc. and the SwiftNIO project authors
// Licensed under Apache License v2.0
//
// See LICENSE.txt for license information
@ -17,8 +17,8 @@ internal struct PriorityQueue<Element: Comparable> {
@usableFromInline
internal var _heap: Heap<Element>
internal init(ascending: Bool = false) {
self._heap = Heap(type: ascending ? .minHeap : .maxHeap)
internal init() {
self._heap = Heap()
}
@inlinable
@ -49,7 +49,7 @@ internal struct PriorityQueue<Element: Comparable> {
@inlinable
internal mutating func clear() {
self._heap = Heap(type: self._heap.type)
self._heap = Heap()
}
}

4
Sources/NIO/SelectableEventLoop.swift
View File

@ -2,7 +2,7 @@
//
// This source file is part of the SwiftNIO open source project
//
// Copyright (c) 2017-2019 Apple Inc. and the SwiftNIO project authors
// Copyright (c) 2017-2020 Apple Inc. and the SwiftNIO project authors
// Licensed under Apache License v2.0
//
// See LICENSE.txt for license information
@ -57,7 +57,7 @@ internal final class SelectableEventLoop: EventLoop {
/* private but tests */ internal let _selector: NIO.Selector<NIORegistration>
private let thread: NIOThread
@usableFromInline
internal var _scheduledTasks = PriorityQueue<ScheduledTask>(ascending: true)
internal var _scheduledTasks = PriorityQueue<ScheduledTask>()
private var tasksCopy = ContiguousArray<() -> Void>()
private let canBeShutdownIndividually: Bool

60
Tests/NIOTests/HeapTests.swift
View File

@ -2,7 +2,7 @@
//
// This source file is part of the SwiftNIO open source project
//
// Copyright (c) 2017-2018 Apple Inc. and the SwiftNIO project authors
// Copyright (c) 2017-2020 Apple Inc. and the SwiftNIO project authors
// Licensed under Apache License v2.0
//
// See LICENSE.txt for license information
@ -23,121 +23,89 @@ public func getRandomNumbers(count: Int) -> [UInt8] {
class HeapTests: XCTestCase {
func testSimple() throws {
var h = Heap<Int>(type: .maxHeap)
h.append(1)
var h = Heap<Int>()
h.append(3)
h.append(1)
h.append(2)
XCTAssertEqual(3, h.removeRoot())
XCTAssertEqual(1, h.removeRoot())
XCTAssertTrue(h.checkHeapProperty())
}
func testSortedDesc() throws {
var maxHeap = Heap<Int>(type: .maxHeap)
var minHeap = Heap<Int>(type: .minHeap)
var minHeap = Heap<Int>()
let input = [16, 14, 10, 9, 8, 7, 4, 3, 2, 1]
input.forEach {
minHeap.append($0)
maxHeap.append($0)
XCTAssertTrue(minHeap.checkHeapProperty())
XCTAssertTrue(maxHeap.checkHeapProperty())
}
var minHeapInputPtr = input.count - 1
var maxHeapInputPtr = 0
while let maxE = maxHeap.removeRoot(), let minE = minHeap.removeRoot() {
XCTAssertEqual(maxE, input[maxHeapInputPtr], "\(maxHeap.debugDescription)")
while let minE = minHeap.removeRoot() {
XCTAssertEqual(minE, input[minHeapInputPtr])
maxHeapInputPtr += 1
minHeapInputPtr -= 1
XCTAssertTrue(minHeap.checkHeapProperty(), "\(minHeap.debugDescription)")
XCTAssertTrue(maxHeap.checkHeapProperty())
}
XCTAssertEqual(-1, minHeapInputPtr)
XCTAssertEqual(input.count, maxHeapInputPtr)
}
func testSortedAsc() throws {
var maxHeap = Heap<Int>(type: .maxHeap)
var minHeap = Heap<Int>(type: .minHeap)
var minHeap = Heap<Int>()
let input = Array([16, 14, 10, 9, 8, 7, 4, 3, 2, 1].reversed())
input.forEach {
minHeap.append($0)
maxHeap.append($0)
}
var minHeapInputPtr = 0
var maxHeapInputPtr = input.count - 1
while let maxE = maxHeap.removeRoot(), let minE = minHeap.removeRoot() {
XCTAssertEqual(maxE, input[maxHeapInputPtr])
while let minE = minHeap.removeRoot() {
XCTAssertEqual(minE, input[minHeapInputPtr])
maxHeapInputPtr -= 1
minHeapInputPtr += 1
}
XCTAssertEqual(input.count, minHeapInputPtr)
XCTAssertEqual(-1, maxHeapInputPtr)
}
func testAddAndRemoveRandomNumbers() throws {
var maxHeap = Heap<UInt8>(type: .maxHeap)
var minHeap = Heap<UInt8>(type: .minHeap)
var maxHeapLast = UInt8.max
var minHeap = Heap<UInt8>()
var minHeapLast = UInt8.min
let N = 10
for n in getRandomNumbers(count: N) {
maxHeap.append(n)
minHeap.append(n)
XCTAssertTrue(maxHeap.checkHeapProperty(), maxHeap.debugDescription)
XCTAssertTrue(minHeap.checkHeapProperty(), maxHeap.debugDescription)
XCTAssertTrue(minHeap.checkHeapProperty(), minHeap.debugDescription)
XCTAssertEqual(Array(minHeap.sorted()), Array(minHeap))
XCTAssertEqual(Array(maxHeap.sorted().reversed()), Array(maxHeap))
}
for _ in 0..<N/2 {
var value = maxHeap.removeRoot()!
XCTAssertLessThanOrEqual(value, maxHeapLast)
maxHeapLast = value
value = minHeap.removeRoot()!
let value = minHeap.removeRoot()!
XCTAssertGreaterThanOrEqual(value, minHeapLast)
minHeapLast = value
XCTAssertTrue(minHeap.checkHeapProperty())
XCTAssertTrue(maxHeap.checkHeapProperty())
XCTAssertEqual(Array(minHeap.sorted()), Array(minHeap))
XCTAssertEqual(Array(maxHeap.sorted().reversed()), Array(maxHeap))
}
maxHeapLast = UInt8.max
minHeapLast = UInt8.min
for n in getRandomNumbers(count: N) {
maxHeap.append(n)
minHeap.append(n)
XCTAssertTrue(maxHeap.checkHeapProperty(), maxHeap.debugDescription)
XCTAssertTrue(minHeap.checkHeapProperty(), maxHeap.debugDescription)
XCTAssertTrue(minHeap.checkHeapProperty(), minHeap.debugDescription)
}
for _ in 0..<N/2+N {
var value = maxHeap.removeRoot()!
XCTAssertLessThanOrEqual(value, maxHeapLast)
maxHeapLast = value
value = minHeap.removeRoot()!
let value = minHeap.removeRoot()!
XCTAssertGreaterThanOrEqual(value, minHeapLast)
minHeapLast = value
XCTAssertTrue(minHeap.checkHeapProperty())
XCTAssertTrue(maxHeap.checkHeapProperty())
}
XCTAssertEqual(0, minHeap.underestimatedCount)
XCTAssertEqual(0, maxHeap.underestimatedCount)
}
func testRemoveElement() throws {
var h = Heap<Int>(type: .maxHeap)
var h = Heap<Int>()
for f in [84, 22, 19, 21, 3, 10, 6, 5, 20] {
h.append(f)
}

1
Tests/NIOTests/PriorityQueueTests+XCTest.swift
View File

@ -30,7 +30,6 @@ extension PriorityQueueTest {
("testSomeStringsAsc", testSomeStringsAsc),
("testRemoveNonExisting", testRemoveNonExisting),
("testRemoveFromEmpty", testRemoveFromEmpty),
("testSomeStringsDesc", testSomeStringsDesc),
("testBuildAndRemoveFromRandomPriorityQueues", testBuildAndRemoveFromRandomPriorityQueues),
("testPartialOrder", testPartialOrder),
("testDescription", testDescription),

81
Tests/NIOTests/PriorityQueueTests.swift
View File

@ -2,7 +2,7 @@
//
// This source file is part of the SwiftNIO open source project
//
// Copyright (c) 2017-2018 Apple Inc. and the SwiftNIO project authors
// Copyright (c) 2017-2020 Apple Inc. and the SwiftNIO project authors
// Licensed under Apache License v2.0
//
// See LICENSE.txt for license information
@ -16,7 +16,7 @@ import XCTest
class PriorityQueueTest: XCTestCase {
func testSomeStringsAsc() throws {
var pq = PriorityQueue<String>(ascending: true)
var pq = PriorityQueue<String>()
pq.push("foo")
pq.push("bar")
pq.push("buz")
@ -39,7 +39,7 @@ class PriorityQueueTest: XCTestCase {
}
func testRemoveNonExisting() throws {
var pq = PriorityQueue<String>(ascending: true)
var pq = PriorityQueue<String>()
pq.push("foo")
pq.remove("bar")
pq.remove("foo")
@ -48,63 +48,38 @@ class PriorityQueueTest: XCTestCase {
}
func testRemoveFromEmpty() throws {
var pq = PriorityQueue<Int>(ascending: true)
var pq = PriorityQueue<Int>()
pq.remove(234)
XCTAssertTrue(pq.isEmpty)
}
func testSomeStringsDesc() throws {
var pq = PriorityQueue<String>(ascending: false)
pq.push("foo")
pq.push("bar")
pq.push("buz")
pq.push("qux")
pq.remove("buz")
XCTAssertEqual("qux", pq.peek()!)
XCTAssertEqual("qux", pq.pop()!)
pq.push("qux")
XCTAssertEqual("qux", pq.peek()!)
XCTAssertEqual("qux", pq.pop()!)
XCTAssertEqual("foo", pq.pop()!)
XCTAssertEqual("bar", pq.pop()!)
XCTAssertTrue(pq.isEmpty)
}
func testBuildAndRemoveFromRandomPriorityQueues() {
for ascending in [true, false] {
for size in 0...33 {
var pq = PriorityQueue<UInt8>(ascending: ascending)
let randoms = getRandomNumbers(count: size)
randoms.forEach { pq.push($0) }
for size in 0...33 {
var pq = PriorityQueue<UInt8>()
let randoms = getRandomNumbers(count: size)
randoms.forEach { pq.push($0) }
/* remove one random member, add it back and assert we're still the same */
randoms.forEach { random in
var pq2 = pq
/* remove one random member, add it back and assert we're still the same */
randoms.forEach { random in
var pq2 = pq
pq2.remove(random)
XCTAssertEqual(pq.count - 1, pq2.count)
XCTAssertNotEqual(pq, pq2)
pq2.push(random)
XCTAssertEqual(pq, pq2)
}
/* remove up to `n` members and add them back at the end and check that the priority queues are still the same */
for n in 1...5 where n <= size {
var pq2 = pq
let deleted = randoms.prefix(n).map { (random: UInt8) -> UInt8 in
pq2.remove(random)
XCTAssertEqual(pq.count - 1, pq2.count)
XCTAssertNotEqual(pq, pq2)
pq2.push(random)
XCTAssertEqual(pq, pq2)
}
/* remove up to `n` members and add them back at the end and check that the priority queues are still the same */
for n in 1...5 where n <= size {
var pq2 = pq
let deleted = randoms.prefix(n).map { (random: UInt8) -> UInt8 in
pq2.remove(random)
return random
}
XCTAssertEqual(pq.count - n, pq2.count)
XCTAssertNotEqual(pq, pq2)
deleted.reversed().forEach { pq2.push($0) }
XCTAssertEqual(pq, pq2, "pq: \(pq), pq2: \(pq2), deleted: \(deleted)")
return random
}
XCTAssertEqual(pq.count - n, pq2.count)
XCTAssertNotEqual(pq, pq2)
deleted.reversed().forEach { pq2.push($0) }
XCTAssertEqual(pq, pq2, "pq: \(pq), pq2: \(pq2), deleted: \(deleted)")
}
}
}
@ -124,7 +99,7 @@ class PriorityQueueTest: XCTestCase {
clearlyTheLargest
*/
var pq = PriorityQueue<SomePartiallyOrderedDataType>(ascending: true)
var pq = PriorityQueue<SomePartiallyOrderedDataType>()
pq.push(clearlyTheLargest)
pq.push(inTheMiddles[0])
pq.push(clearlyTheSmallest)