Simplify Heap implementation. (#960)

Motivation:

Our original Heap implementation used a bunch of static functions
in order to make it clear how it related to textbook heap
implementations. This was primarily done to ensure that it was easier
to see the correctness of the code.

However, we've long been satisfied with the correctness of the code,
and the Heap is one of the best tested parts of NIO. For this reason,
we should be free to refactor it.

Given https://bugs.swift.org/browse/SR-10346, we've been given an
incentive to do that refactor right now. This is because the Heap
is causing repeated CoW operations each time we enqueue new work
onto the event loop. That's a substantial performance cost: well
worth fixing with a small rewrite.

Modifications:

- Removed all static funcs from Heap
- Rewrote some into instance funcs
- Merged the implementation of insert into append.
- Added an allocation test to avoid regressing this change.

Result:

Faster Heap, happier users, sadder textbook authors.

IntegrationTests/tests_04_performance/test_01_allocation_counts_for_http1.sh

@@ -58,7 +58,7 @@ cd ..
 "$swift_bin" run -c release | tee "$tmp/output"
 )
 
-for test in 1000_reqs_1_conn 1_reqs_1000_conn ping_pong_1000_reqs_1_conn bytebuffer_lots_of_rw future_lots_of_callbacks; do
+for test in 1000_reqs_1_conn 1_reqs_1000_conn ping_pong_1000_reqs_1_conn bytebuffer_lots_of_rw future_lots_of_callbacks scheduling_10000_executions; do
     cat "$tmp/output"  # helps debugging
     total_allocations=$(grep "^$test.total_allocations:" "$tmp/output" | cut -d: -f2 | sed 's/ //g')
     not_freed_allocations=$(grep "^$test.remaining_allocations:" "$tmp/output" | cut -d: -f2 | sed 's/ //g')

IntegrationTests/tests_04_performance/test_01_resources/template/Sources/SwiftBootstrap/SwiftMain.swift

@@ -18,7 +18,7 @@ import NIO
 import NIOHTTP1
 import Foundation
 import AtomicCounter
-import Dispatch // needed for Swift 4.0 on Linux only
+import Dispatch
 
 private final class SimpleHTTPServer: ChannelInboundHandler {
     typealias InboundIn = HTTPServerRequestPart
@@ -436,5 +436,21 @@ public func swiftMain() -> Int {
         return 1000
     }
 
+    measureAndPrint(desc: "scheduling_10000_executions") {
+        let group = MultiThreadedEventLoopGroup(numberOfThreads: 1)
+        let loop = group.next()
+        let dg = DispatchGroup()
+
+        try! loop.submit {
+            for _ in 0..<10_000 {
+                dg.enter()
+                loop.execute { dg.leave() }
+            }
+        }.wait()
+        dg.wait()
+        try! group.syncShutdownGracefully()
+        return 10_000
+    }
+
     return 0
 }

Sources/NIO/Heap.swift

@@ -51,76 +51,64 @@ internal struct Heap<T: Comparable> {
         self.type = type
     }
 
-    // MARK: verbatim from CLRS's (Introduction to Algorithms) Heapsort chapter with the following changes
-    //  - added a `compare` parameter to make it either a min or a max heap
-    //  - renamed `largest` to `root`
-    //  - removed `max` from the method names like `maxHeapify`, `maxHeapInsert` etc
-    //  - made the arrays 0-based
-
     // named `PARENT` in CLRS
-    private static func parentIndex(_ i: Int) -> Int {
+    private func parentIndex(_ i: Int) -> Int {
         return (i-1) / 2
     }
 
     // named `LEFT` in CLRS
-    private static func leftIndex(_ i: Int) -> Int {
+    private func leftIndex(_ i: Int) -> Int {
         return 2*i + 1
     }
 
     // named `RIGHT` in CLRS
-    private static func rightIndex(_ i: Int) -> Int {
+    private func rightIndex(_ i: Int) -> Int {
         return 2*i + 2
     }
 
     // named `MAX-HEAPIFY` in CLRS
-    private static func heapify(storage: inout ContiguousArray<T>, compare: (T, T) -> Bool, _ i: Int) {
+    private mutating func heapify(_ index: Int) {
-        let l = Heap<T>.leftIndex(i)
+        let left = self.leftIndex(index)
-        let r = Heap<T>.rightIndex(i)
+        let right = self.rightIndex(index)
 
         var root: Int
-        if l <= (storage.count - 1) && compare(storage[l], storage[i]) {
+        if left <= (self.storage.count - 1) && self.comparator(storage[left], storage[index]) {
-            root = l
+            root = left
         } else {
-            root = i
+            root = index
         }
 
-        if r <= (storage.count - 1) && compare(storage[r], storage[root]) {
+        if right <= (self.storage.count - 1) && self.comparator(storage[right], storage[root]) {
-            root = r
+            root = right
         }
 
-        if root != i {
+        if root != index {
-            storage.swapAt(i, root)
+            self.storage.swapAt(index, root)
-            heapify(storage: &storage, compare: compare, root)
+            self.heapify(root)
-        }
-    }
-
-    // named `MAX-HEAP-INSERT` in CRLS
-    private static func heapInsert(storage: inout ContiguousArray<T>, compare: (T, T) -> Bool, key: T) {
-        var i = storage.count
-        storage.append(key)
-        while i > 0 && compare(storage[i], storage[parentIndex(i)]) {
-            storage.swapAt(i, parentIndex(i))
-            i = parentIndex(i)
         }
     }
 
     // named `HEAP-INCREASE-KEY` in CRLS
-    private static func heapRootify(storage: inout ContiguousArray<T>, compare: (T, T) -> Bool, index: Int, key: T) {
+    private mutating func heapRootify(index: Int, key: T) {
         var index = index
-        if compare(storage[index], key) {
+        if self.comparator(storage[index], key) {
             fatalError("New key must be closer to the root than current key")
         }
 
-        storage[index] = key
+        self.storage[index] = key
-        while index > 0 && compare(storage[index], storage[parentIndex(index)]) {
+        while index > 0 && self.comparator(self.storage[index], self.storage[self.parentIndex(index)]) {
-            storage.swapAt(index, parentIndex(index))
+            self.storage.swapAt(index, self.parentIndex(index))
-            index = parentIndex(index)
+            index = self.parentIndex(index)
         }
     }
 
-    // MARK: Swift interface using the low-level methods above
     public mutating func append(_ value: T) {
-        Heap<T>.heapInsert(storage: &self.storage, compare: self.comparator, key: value)
+        var i = self.storage.count
+        self.storage.append(value)
+        while i > 0 && self.comparator(self.storage[i], self.storage[self.parentIndex(i)]) {
+            self.storage.swapAt(i, self.parentIndex(i))
+            i = self.parentIndex(i)
+        }
     }
 
     @discardableResult
@@ -144,23 +132,24 @@ internal struct Heap<T: 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 !self.comparator(self.storage[index], self.storage[self.storage.count - 1]) {
+        } else if !comparator(self.storage[index], self.storage[self.storage.count - 1]) {
-            Heap<T>.heapRootify(storage: &self.storage, compare: self.comparator, index: index, key: self.storage[self.storage.count - 1])
+            self.heapRootify(index: index, key: self.storage[self.storage.count - 1])
             self.storage.removeLast()
         } else {
             self.storage[index] = self.storage[self.storage.count - 1]
             self.storage.removeLast()
-            Heap<T>.heapify(storage: &self.storage, compare: self.comparator, index)
+            self.heapify(index)
         }
         return element
     }
 
     internal func checkHeapProperty() -> Bool {
         func checkHeapProperty(index: Int) -> Bool {
-            let li = Heap<T>.leftIndex(index)
+            let li = self.leftIndex(index)
-            let ri = Heap<T>.rightIndex(index)
+            let ri = self.rightIndex(index)
             if index >= self.storage.count {
                 return true
             } else {
@@ -204,8 +193,8 @@ extension Heap: CustomDebugStringConvertible {
         var all = "\n"
         let spacing = String(repeating: " ", count: maxLen)
         func subtreeWidths(rootIndex: Int) -> (Int, Int) {
-            let lcIdx = Heap<T>.leftIndex(rootIndex)
+            let lcIdx = self.leftIndex(rootIndex)
-            let rcIdx = Heap<T>.rightIndex(rootIndex)
+            let rcIdx = self.rightIndex(rootIndex)
             var leftSpace = 0
             var rightSpace = 0
             if lcIdx < self.storage.count {

docker/docker-compose.1804.50.yaml

@@ -18,20 +18,22 @@ services:
     image: swift-nio:18.04-5.0
     environment:
       - MAX_ALLOCS_ALLOWED_1000_reqs_1_conn=31200
-      - MAX_ALLOCS_ALLOWED_1_reqs_1000_conn=1177050 # was: 685050
+      - MAX_ALLOCS_ALLOWED_1_reqs_1000_conn=1155050 # was: 685050
       - MAX_ALLOCS_ALLOWED_ping_pong_1000_reqs_1_conn=4600
       - MAX_ALLOCS_ALLOWED_bytebuffer_lots_of_rw=2100
       - MAX_ALLOCS_ALLOWED_future_lots_of_callbacks=99100
+      - MAX_ALLOCS_ALLOWED_scheduling_10000_executions=20150
 
   test:
     image: swift-nio:18.04-5.0
     command: /bin/bash -cl "swift test -Xswiftc -warnings-as-errors && ./scripts/integration_tests.sh"
     environment:
       - MAX_ALLOCS_ALLOWED_1000_reqs_1_conn=31200
-      - MAX_ALLOCS_ALLOWED_1_reqs_1000_conn=1177050 # was: 685050
+      - MAX_ALLOCS_ALLOWED_1_reqs_1000_conn=1155050 # was: 685050
       - MAX_ALLOCS_ALLOWED_ping_pong_1000_reqs_1_conn=4600
       - MAX_ALLOCS_ALLOWED_bytebuffer_lots_of_rw=2100
       - MAX_ALLOCS_ALLOWED_future_lots_of_callbacks=99100
+      - MAX_ALLOCS_ALLOWED_scheduling_10000_executions=20150
 
   echo:
     image: swift-nio:18.04-5.0