From 88513d75cfa97fb42f0d820ec89dfc968276ee9f Mon Sep 17 00:00:00 2001 From: Brennan Saeta Date: Sat, 23 May 2020 16:18:21 -0700 Subject: [PATCH 1/8] Refactor co-primes algorithm to be generic & avoid allocation. Thank you @dabrahams for the suggestions! --- .../NonBlockingThreadPool.swift | 2 +- .../NumberOperations.swift | 19 ----- .../PenguinStructures/NumberOperations.swift | 72 +++++++++++++++++++ .../NumberOperationsTests.swift | 29 ++++++++ .../XCTestManifests.swift | 1 + 5 files changed, 103 insertions(+), 20 deletions(-) create mode 100644 Sources/PenguinStructures/NumberOperations.swift create mode 100644 Tests/PenguinStructuresTests/NumberOperationsTests.swift diff --git a/Sources/PenguinParallel/NonblockingThreadPool/NonBlockingThreadPool.swift b/Sources/PenguinParallel/NonblockingThreadPool/NonBlockingThreadPool.swift index 9fe54f58..f11854ad 100644 --- a/Sources/PenguinParallel/NonblockingThreadPool/NonBlockingThreadPool.swift +++ b/Sources/PenguinParallel/NonblockingThreadPool/NonBlockingThreadPool.swift @@ -99,7 +99,7 @@ public class NonBlockingThreadPool: ComputeThr let totalThreadCount = threadCount + externalFastPathThreadCount self.totalThreadCount = totalThreadCount self.externalFastPathThreadCount = externalFastPathThreadCount - self.coprimes = positiveCoprimes(totalThreadCount) + self.coprimes = Array(totalThreadCount.positiveCoprimes) self.queues = (0.. [Int] { - var coprimes = [Int]() - for i in 1...n { - var a = i - var b = n - // If GCD(a, b) == 1, then a and b are coprimes. - while b != 0 { - let tmp = a - a = b - b = tmp % b - } - if a == 1 { coprimes.append(i) } - } - return coprimes -} - /// Returns a value deterministically selected from `0.. { PositiveCoprimes(self) } +} + +/// A sequence of numbers that are co-prime with `n`, up to `n`. +public struct PositiveCoprimes: Sequence { + /// The number to find co-primes relative to. + let n: Number + + /// Constructs a `PositiveCoprimes` sequence of numbers co-prime relative to `n`. + internal init(_ n: Number) { + precondition(n > 0, "\(n) doees not have defined positive co-primes.") + self.n = n + } + + /// Returns an iterator that incrementally computes co-primes relative to `n`. + public func makeIterator() -> Iterator { + Iterator(n: n, i: 0) + } + + /// Iteratively computes co-primes relative to `n` starting from 1. + public struct Iterator: IteratorProtocol { + /// The number we are finding co-primes relative to. + let n: Number + /// A sequence counter representing one less than the next candidate to try. + var i: Number + + /// Returns the next co-prime, or nil if all co-primes have been found. + mutating public func next() -> Number? { + while (i+1) < n { + i += 1 + if greatestCommonDivisor(i, n) == 1 { return i } + } + return nil + } + } +} + +/// Returns the greatest common divisor between two numbers. +/// +/// This implementation uses Euclid's algorithm. +// TODO: Switch to the Binary GCD algorithm which avoids expensive modulo operations. +public func greatestCommonDivisor(_ a: Number, _ b: Number) -> Number { + var a = a + var b = b + if a > b { + swap(&a, &b) + } + while b != 0 { + let tmp = a + a = b + b = tmp % b + } + return a +} diff --git a/Tests/PenguinStructuresTests/NumberOperationsTests.swift b/Tests/PenguinStructuresTests/NumberOperationsTests.swift new file mode 100644 index 00000000..5143e98f --- /dev/null +++ b/Tests/PenguinStructuresTests/NumberOperationsTests.swift @@ -0,0 +1,29 @@ +// Copyright 2020 Penguin Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import PenguinStructures +import XCTest + +final class NumberOperationsTests: XCTestCase { + func testCoprimes() { + XCTAssertEqual([1, 3, 5, 7], Array(8.positiveCoprimes)) + XCTAssertEqual([1, 2, 3, 4], Array(5.positiveCoprimes)) + XCTAssertEqual([1, 2, 4, 7, 8, 11, 13, 14], Array(15.positiveCoprimes)) + XCTAssertEqual([], Array(1.positiveCoprimes)) + } + + static var allTests = [ + ("testCoprimes", testCoprimes), + ] +} diff --git a/Tests/PenguinStructuresTests/XCTestManifests.swift b/Tests/PenguinStructuresTests/XCTestManifests.swift index 27abeb95..a85a3c85 100644 --- a/Tests/PenguinStructuresTests/XCTestManifests.swift +++ b/Tests/PenguinStructuresTests/XCTestManifests.swift @@ -25,6 +25,7 @@ import XCTest testCase(ArrayStorageTests.allTests), testCase(ArrayStorageExtensionTests.allTests), testCase(ArrayBufferTests.allTests), + testCase(NumberOperationsTests.allTests), ] } #endif From 9dbb65c7a7232c92e21cfb9bdd10719c8749063a Mon Sep 17 00:00:00 2001 From: Brennan Saeta Date: Sun, 24 May 2020 23:57:11 -0700 Subject: [PATCH 2/8] Respond to comments. --- .../NonBlockingThreadPool.swift | 14 +++- .../PenguinStructures/NumberOperations.swift | 78 ++++++++++++------- .../NumberOperationsTests.swift | 26 ++++++- 3 files changed, 83 insertions(+), 35 deletions(-) diff --git a/Sources/PenguinParallel/NonblockingThreadPool/NonBlockingThreadPool.swift b/Sources/PenguinParallel/NonblockingThreadPool/NonBlockingThreadPool.swift index f11854ad..41ae14b2 100644 --- a/Sources/PenguinParallel/NonblockingThreadPool/NonBlockingThreadPool.swift +++ b/Sources/PenguinParallel/NonblockingThreadPool/NonBlockingThreadPool.swift @@ -64,7 +64,13 @@ public class NonBlockingThreadPool: ComputeThr let totalThreadCount: Int let externalFastPathThreadCount: Int var externalFastPathThreadSeenCount: Int = 0 - let coprimes: [Int] + + /// An array of step-sizes that are each coprime with `queues.count`. + /// + /// When looking for work, pool threads will pick a random step size and traverse the `queues` + /// looking to steal work. The coprime property ensures that every queue will be examined, but the + /// stealing threads will traverse in diverging orders, avoiding thundering heards. + let stepSizes: [Int] let queues: [Queue] var cancelledStorage: AtomicUInt64 var blockedCountStorage: AtomicUInt64 @@ -99,7 +105,7 @@ public class NonBlockingThreadPool: ComputeThr let totalThreadCount = threadCount + externalFastPathThreadCount self.totalThreadCount = totalThreadCount self.externalFastPathThreadCount = externalFastPathThreadCount - self.coprimes = Array(totalThreadCount.positiveCoprimes) + self.stepSizes = totalThreadCount.smallerPositiveCoprimes self.queues = (0.. { func steal() -> Task? { let r = Int(rng.next()) var selectedThreadId = fastFit(r, into: pool.totalThreadCount) - let step = pool.coprimes[fastFit(r, into: pool.coprimes.count)] + let step = pool.stepSizes[fastFit(r, into: pool.stepSizes.count)] assert( step < pool.totalThreadCount, "step: \(step), pool threadcount: \(pool.totalThreadCount)") @@ -492,7 +498,7 @@ fileprivate final class PerThreadState { private func findNonEmptyQueueIndex() -> Int? { let r = Int(rng.next()) let increment = - pool.totalThreadCount == 1 ? 1 : pool.coprimes[fastFit(r, into: pool.coprimes.count)] + pool.totalThreadCount == 1 ? 1 : pool.stepSizes[fastFit(r, into: pool.stepSizes.count)] var threadIndex = fastFit(r, into: pool.totalThreadCount) for _ in 0.. { PositiveCoprimes(self) } + /// Definition: Two numbers are coprime if their GCD is 1. + public var positiveCoprimes: PositiveCoprimes { .init(self) } + + /// Returns positive integers that are coprime with, and smaller than, `self`. + /// + /// - SeeAlso: `positiveCoprimes`. + public var smallerPositiveCoprimes: [Self] { + var positiveSelf = self + if positiveSelf < 0 { + positiveSelf *= -1 // Workaround for lack of `Swift.abs` on `BinaryInteger`. + } + return positiveCoprimes.prefix { $0 < positiveSelf } + } } -/// A sequence of numbers that are co-prime with `n`, up to `n`. -public struct PositiveCoprimes: Sequence { - /// The number to find co-primes relative to. - let n: Number +/// The positive values that are coprime with *N*. +public struct PositiveCoprimes: Sequence { + /// The number to find coprimes relative to. + let target: Domain - /// Constructs a `PositiveCoprimes` sequence of numbers co-prime relative to `n`. - internal init(_ n: Number) { - precondition(n > 0, "\(n) doees not have defined positive co-primes.") - self.n = n + /// Constructs a `PositiveCoprimes` sequence of numbers coprime relative to `n`. + internal init(_ target: Domain) { + var target = target + if target < 0 { + target = target * -1 // Make positive; Swift.abs is unavailable. + } + self.target = target } - /// Returns an iterator that incrementally computes co-primes relative to `n`. + /// Returns an iterator that incrementally computes coprimes relative to `n`. public func makeIterator() -> Iterator { - Iterator(n: n, i: 0) + Iterator(target: target) } - /// Iteratively computes co-primes relative to `n` starting from 1. + /// Iteratively computes coprimes relative to `n` starting from 1. public struct Iterator: IteratorProtocol { - /// The number we are finding co-primes relative to. - let n: Number - /// A sequence counter representing one less than the next candidate to try. - var i: Number + /// The number we are finding coprimes relative to. + let target: Domain + /// The next candidate to test for relative primality. + var nextCandidate: Domain = 1 - /// Returns the next co-prime, or nil if all co-primes have been found. - mutating public func next() -> Number? { - while (i+1) < n { - i += 1 - if greatestCommonDivisor(i, n) == 1 { return i } + /// Returns the next positive coprime, or nil if no coprimes are defined. + mutating public func next() -> Domain? { + if _slowPath(target == 0) { return nil } // Nothing is coprime with 0. + while true { + let candidate = nextCandidate + nextCandidate += 1 + if gcd(candidate, target) == 1 { return candidate } } - return nil } } } -/// Returns the greatest common divisor between two numbers. +/// Returns the greatest common divisor of `a` and `b`. /// -/// This implementation uses Euclid's algorithm. +/// - Complexity: O(n^2) where `n` is the number of bits in `Domain`. // TODO: Switch to the Binary GCD algorithm which avoids expensive modulo operations. -public func greatestCommonDivisor(_ a: Number, _ b: Number) -> Number { +public func gcd(_ a: Domain, _ b: Domain) -> Domain { var a = a var b = b + + if a < 0 { + a *= -1 + } + + if b < 0 { + b *= -1 + } + if a > b { swap(&a, &b) } diff --git a/Tests/PenguinStructuresTests/NumberOperationsTests.swift b/Tests/PenguinStructuresTests/NumberOperationsTests.swift index 5143e98f..37c66c60 100644 --- a/Tests/PenguinStructuresTests/NumberOperationsTests.swift +++ b/Tests/PenguinStructuresTests/NumberOperationsTests.swift @@ -17,13 +17,31 @@ import XCTest final class NumberOperationsTests: XCTestCase { func testCoprimes() { - XCTAssertEqual([1, 3, 5, 7], Array(8.positiveCoprimes)) - XCTAssertEqual([1, 2, 3, 4], Array(5.positiveCoprimes)) - XCTAssertEqual([1, 2, 4, 7, 8, 11, 13, 14], Array(15.positiveCoprimes)) - XCTAssertEqual([], Array(1.positiveCoprimes)) + XCTAssertEqual([1, 3, 5, 7], 8.smallerPositiveCoprimes) + XCTAssertEqual([1, 2, 3, 4], 5.smallerPositiveCoprimes) + XCTAssertEqual([1, 2, 4, 7, 8, 11, 13, 14], 15.smallerPositiveCoprimes) + XCTAssertEqual([], 1.smallerPositiveCoprimes) + + XCTAssertEqual([1, 3, 5, 7], (-8).smallerPositiveCoprimes) + + XCTAssertEqual([1, 3, 5, 7, 9, 11, 13, 15], Array(8.positiveCoprimes.prefix(8))) + XCTAssertEqual(Array(1...8), Array(1.positiveCoprimes.prefix(8))) + + XCTAssertEqual([], Array(0.positiveCoprimes.prefix(8))) + } + + func testGCD() { + XCTAssertEqual(3, gcd(0, 3)) + XCTAssertEqual(3, gcd(3, 0)) + XCTAssertEqual(4, gcd(-4, 8)) + + XCTAssertEqual(3, gcd(UInt(15), 3)) + + XCTAssertEqual(5, gcd(-5, 0)) } static var allTests = [ ("testCoprimes", testCoprimes), + ("testGCD", testGCD), ] } From b64f1736d09c553e4d43b3b474dc344e8353c5bc Mon Sep 17 00:00:00 2001 From: Brennan Saeta Date: Sun, 7 Jun 2020 01:50:16 -0700 Subject: [PATCH 3/8] Make `PositiveCoprimes` a `Collection` instead of a `Sequence`. --- .../PenguinStructures/NumberOperations.swift | 64 +++++++++++++------ 1 file changed, 44 insertions(+), 20 deletions(-) diff --git a/Sources/PenguinStructures/NumberOperations.swift b/Sources/PenguinStructures/NumberOperations.swift index 0a1d2d25..4dca4635 100644 --- a/Sources/PenguinStructures/NumberOperations.swift +++ b/Sources/PenguinStructures/NumberOperations.swift @@ -13,7 +13,7 @@ // limitations under the License. extension BinaryInteger { - /// Returns a sequence of the positive integers that are coprime with `self`. + /// Returns a collection of the positive integers that are coprime with `self`. /// /// Definition: Two numbers are coprime if their GCD is 1. public var positiveCoprimes: PositiveCoprimes { .init(self) } @@ -31,9 +31,23 @@ extension BinaryInteger { } /// The positive values that are coprime with *N*. -public struct PositiveCoprimes: Sequence { +/// +/// Example: +/// ``` +/// print(Array(10.positiveCoprimes.prefix(8))) // [1, 3, 7, 9, 11, 13, 17, 19] +/// ``` +/// +/// Note: Although there are infinitely many positive prime numbers, `PositiveCoprimes` is bounded +/// by the maximum representable integer in `Domain`. +// TODO: Specialize SubSequence for efficiency. +public struct PositiveCoprimes: Collection { /// The number to find coprimes relative to. - let target: Domain + public let target: Domain + + /// The index into the collection of positive coprimes are the coprimes themselves. + /// + /// Note: the indices are not dense or contiguous in `Domain`. + public typealias Index = Domain /// Constructs a `PositiveCoprimes` sequence of numbers coprime relative to `n`. internal init(_ target: Domain) { @@ -44,26 +58,36 @@ public struct PositiveCoprimes: Sequence { self.target = target } - /// Returns an iterator that incrementally computes coprimes relative to `n`. - public func makeIterator() -> Iterator { - Iterator(target: target) + /// `Int.max`, as there are infinitely many prime numbers, and thus infinitely many coprimes + /// to a given target. + public var count: Int { + if _slowPath(target == 0) { return 0 } + return Int.max } - /// Iteratively computes coprimes relative to `n` starting from 1. - public struct Iterator: IteratorProtocol { - /// The number we are finding coprimes relative to. - let target: Domain - /// The next candidate to test for relative primality. - var nextCandidate: Domain = 1 + /// Accesses the coprime at `index`. + public subscript(index: Index) -> Domain { + index + } + + /// The first valid coprime (1). + public var startIndex: Index { 1 } + + /// The largest positive reasonable coprime. + /// + /// Note: if a `BinaryInteger` larger than `UInt64.max` is used, the `endIndex` might leave off + /// potentially useful integers. + public var endIndex: Index { + if _slowPath(target == 0) { return startIndex } + return Domain(clamping: UInt64.max) + } - /// Returns the next positive coprime, or nil if no coprimes are defined. - mutating public func next() -> Domain? { - if _slowPath(target == 0) { return nil } // Nothing is coprime with 0. - while true { - let candidate = nextCandidate - nextCandidate += 1 - if gcd(candidate, target) == 1 { return candidate } - } + /// Computes the next index after `index`. + public func index(after index: Index) -> Index { + var nextCandidate = index + while true { + nextCandidate += 1 + if gcd(nextCandidate, target) == 1 { return nextCandidate } } } } From b97c1c2ac8e4874f87c1d7ea111266d98c345d6e Mon Sep 17 00:00:00 2001 From: Brennan Saeta Date: Fri, 19 Jun 2020 13:04:04 -0700 Subject: [PATCH 4/8] Apply suggestions from code review Co-authored-by: Dave Abrahams --- .../NonblockingThreadPool/NonBlockingThreadPool.swift | 4 ++-- Sources/PenguinStructures/NumberOperations.swift | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Sources/PenguinParallel/NonblockingThreadPool/NonBlockingThreadPool.swift b/Sources/PenguinParallel/NonblockingThreadPool/NonBlockingThreadPool.swift index ed69ea6b..d3a90aa8 100644 --- a/Sources/PenguinParallel/NonblockingThreadPool/NonBlockingThreadPool.swift +++ b/Sources/PenguinParallel/NonblockingThreadPool/NonBlockingThreadPool.swift @@ -69,7 +69,7 @@ public class NonBlockingThreadPool: ComputeThr /// /// When looking for work, pool threads will pick a random step size and traverse the `queues` /// looking to steal work. The coprime property ensures that every queue will be examined, but the - /// stealing threads will traverse in diverging orders, avoiding thundering heards. + /// stealing threads will traverse in diverging orders, avoiding thundering herds. let stepSizes: [Int] let queues: [Queue] var cancelledStorage: AtomicUInt64 @@ -105,7 +105,7 @@ public class NonBlockingThreadPool: ComputeThr let totalThreadCount = threadCount + externalFastPathThreadCount self.totalThreadCount = totalThreadCount self.externalFastPathThreadCount = externalFastPathThreadCount - self.stepSizes = totalThreadCount.smallerPositiveCoprimes + self.stepSizes = totalThreadCount.lesserPositiveCoprimes self.queues = (0..: Collection { /// Note: the indices are not dense or contiguous in `Domain`. public typealias Index = Domain - /// Constructs a `PositiveCoprimes` sequence of numbers coprime relative to `n`. + /// Creates a collection of numbers coprime relative to `n`. internal init(_ target: Domain) { var target = target if target < 0 { From 4a6e40c24e00d3bda8ff2061be98f91cb0937ec8 Mon Sep 17 00:00:00 2001 From: Brennan Saeta Date: Fri, 19 Jun 2020 13:36:04 -0700 Subject: [PATCH 5/8] Respond to review comments. --- .../PenguinStructures/NumberOperations.swift | 24 +++++++------------ .../NumberOperationsTests.swift | 11 +++++---- 2 files changed, 15 insertions(+), 20 deletions(-) diff --git a/Sources/PenguinStructures/NumberOperations.swift b/Sources/PenguinStructures/NumberOperations.swift index 54cd187c..3c8edc23 100644 --- a/Sources/PenguinStructures/NumberOperations.swift +++ b/Sources/PenguinStructures/NumberOperations.swift @@ -18,15 +18,11 @@ extension BinaryInteger { /// Definition: Two numbers are coprime if their GCD is 1. public var positiveCoprimes: PositiveCoprimes { .init(self) } - /// Returns positive integers that are coprime with, and smaller than, `self`. + /// Returns positive integers that are coprime with, and less than, `self`. /// /// - SeeAlso: `positiveCoprimes`. - public var smallerPositiveCoprimes: [Self] { - var positiveSelf = self - if positiveSelf < 0 { - positiveSelf *= -1 // Workaround for lack of `Swift.abs` on `BinaryInteger`. - } - return positiveCoprimes.prefix { $0 < positiveSelf } + public var lesserPositiveCoprimes: [Self] { + return positiveCoprimes.prefix { $0 < self } } } @@ -38,9 +34,11 @@ extension BinaryInteger { /// ``` /// /// Note: Although there are infinitely many positive prime numbers, `PositiveCoprimes` is bounded -/// by the maximum representable integer in `Domain`. -// TODO: Specialize SubSequence for efficiency. +/// by the maximum representable integer in `Domain` (if such a limit exists, such as the +/// `FixedWidthInteger` types). public struct PositiveCoprimes: Collection { + // TODO: Specialize SubSequence for efficiency. + /// The number to find coprimes relative to. public let target: Domain @@ -49,13 +47,9 @@ public struct PositiveCoprimes: Collection { /// Note: the indices are not dense or contiguous in `Domain`. public typealias Index = Domain - /// Creates a collection of numbers coprime relative to `n`. + /// Creates a collection of numbers coprime relative to `target`. internal init(_ target: Domain) { - var target = target - if target < 0 { - target = target * -1 // Make positive; Swift.abs is unavailable. - } - self.target = target + self.target = target < 0 ? target * -1 : target } /// `Int.max`, as there are infinitely many prime numbers, and thus infinitely many coprimes diff --git a/Tests/PenguinStructuresTests/NumberOperationsTests.swift b/Tests/PenguinStructuresTests/NumberOperationsTests.swift index 37c66c60..f59c34a4 100644 --- a/Tests/PenguinStructuresTests/NumberOperationsTests.swift +++ b/Tests/PenguinStructuresTests/NumberOperationsTests.swift @@ -17,14 +17,15 @@ import XCTest final class NumberOperationsTests: XCTestCase { func testCoprimes() { - XCTAssertEqual([1, 3, 5, 7], 8.smallerPositiveCoprimes) - XCTAssertEqual([1, 2, 3, 4], 5.smallerPositiveCoprimes) - XCTAssertEqual([1, 2, 4, 7, 8, 11, 13, 14], 15.smallerPositiveCoprimes) - XCTAssertEqual([], 1.smallerPositiveCoprimes) + XCTAssertEqual([1, 3, 5, 7], 8.lesserPositiveCoprimes) + XCTAssertEqual([1, 2, 3, 4], 5.lesserPositiveCoprimes) + XCTAssertEqual([1, 2, 4, 7, 8, 11, 13, 14], 15.lesserPositiveCoprimes) + XCTAssertEqual([], 1.lesserPositiveCoprimes) - XCTAssertEqual([1, 3, 5, 7], (-8).smallerPositiveCoprimes) + XCTAssertEqual([], (-8).lesserPositiveCoprimes) XCTAssertEqual([1, 3, 5, 7, 9, 11, 13, 15], Array(8.positiveCoprimes.prefix(8))) + XCTAssertEqual([1, 3, 5, 7, 9, 11, 13, 15], Array((-8).positiveCoprimes.prefix(8))) XCTAssertEqual(Array(1...8), Array(1.positiveCoprimes.prefix(8))) XCTAssertEqual([], Array(0.positiveCoprimes.prefix(8))) From db9eaeb33369f902b5db977d4ffe2df9b31d15e5 Mon Sep 17 00:00:00 2001 From: Brennan Saeta Date: Fri, 19 Jun 2020 13:40:31 -0700 Subject: [PATCH 6/8] Make `endIndex` documentation match implementation. --- Sources/PenguinStructures/NumberOperations.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/PenguinStructures/NumberOperations.swift b/Sources/PenguinStructures/NumberOperations.swift index 3c8edc23..e6aab838 100644 --- a/Sources/PenguinStructures/NumberOperations.swift +++ b/Sources/PenguinStructures/NumberOperations.swift @@ -67,7 +67,7 @@ public struct PositiveCoprimes: Collection { /// The first valid coprime (1). public var startIndex: Index { 1 } - /// The largest positive reasonable coprime. + /// The largest possible element of `Domain` up to `UInt64.max`. /// /// Note: if a `BinaryInteger` larger than `UInt64.max` is used, the `endIndex` might leave off /// potentially useful integers. From cc7a4db68b3bac6cebd7704c8852f42e88cbf25c Mon Sep 17 00:00:00 2001 From: Brennan Saeta Date: Fri, 10 Jul 2020 10:06:22 -0700 Subject: [PATCH 7/8] Apply suggestions from code review Co-authored-by: Dave Abrahams --- .../NonBlockingThreadPool.swift | 4 ++-- .../PenguinStructures/NumberOperations.swift | 22 ++++++++----------- 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/Sources/PenguinParallel/NonblockingThreadPool/NonBlockingThreadPool.swift b/Sources/PenguinParallel/NonblockingThreadPool/NonBlockingThreadPool.swift index d3a90aa8..0046ef88 100644 --- a/Sources/PenguinParallel/NonblockingThreadPool/NonBlockingThreadPool.swift +++ b/Sources/PenguinParallel/NonblockingThreadPool/NonBlockingThreadPool.swift @@ -65,12 +65,12 @@ public class NonBlockingThreadPool: ComputeThr let externalFastPathThreadCount: Int var externalFastPathThreadSeenCount: Int = 0 - /// An array of step-sizes that are each coprime with `queues.count`. + /// A vocabulary of step sizes used to randomly search the list of queues for work to steal. /// /// When looking for work, pool threads will pick a random step size and traverse the `queues` /// looking to steal work. The coprime property ensures that every queue will be examined, but the /// stealing threads will traverse in diverging orders, avoiding thundering herds. - let stepSizes: [Int] + let workStealingStrides: [Int] let queues: [Queue] var cancelledStorage: AtomicUInt64 var blockedCountStorage: AtomicUInt64 diff --git a/Sources/PenguinStructures/NumberOperations.swift b/Sources/PenguinStructures/NumberOperations.swift index e6aab838..5373c95a 100644 --- a/Sources/PenguinStructures/NumberOperations.swift +++ b/Sources/PenguinStructures/NumberOperations.swift @@ -13,7 +13,7 @@ // limitations under the License. extension BinaryInteger { - /// Returns a collection of the positive integers that are coprime with `self`. + /// A collection of the positive integers that are coprime with `self`. /// /// Definition: Two numbers are coprime if their GCD is 1. public var positiveCoprimes: PositiveCoprimes { .init(self) } @@ -49,7 +49,7 @@ public struct PositiveCoprimes: Collection { /// Creates a collection of numbers coprime relative to `target`. internal init(_ target: Domain) { - self.target = target < 0 ? target * -1 : target + self.target = target < 0 ? -target : target } /// `Int.max`, as there are infinitely many prime numbers, and thus infinitely many coprimes @@ -60,11 +60,9 @@ public struct PositiveCoprimes: Collection { } /// Accesses the coprime at `index`. - public subscript(index: Index) -> Domain { - index - } + public subscript(index: Index) -> Domain { index } - /// The first valid coprime (1). + /// The index of the first element. public var startIndex: Index { 1 } /// The largest possible element of `Domain` up to `UInt64.max`. @@ -76,8 +74,8 @@ public struct PositiveCoprimes: Collection { return Domain(clamping: UInt64.max) } - /// Computes the next index after `index`. - public func index(after index: Index) -> Index { + /// Returns the position after `i`. + public func index(after i: Index) -> Index { var nextCandidate = index while true { nextCandidate += 1 @@ -95,20 +93,18 @@ public func gcd(_ a: Domain, _ b: Domain) -> Domain { var b = b if a < 0 { - a *= -1 + a = -a } if b < 0 { - b *= -1 + b = -b } if a > b { swap(&a, &b) } while b != 0 { - let tmp = a - a = b - b = tmp % b + (a, b) = (b, a % b) } return a } From 5bfda61072d3ba1032e9b15ab4e2824f1a00d4ab Mon Sep 17 00:00:00 2001 From: Brennan Saeta Date: Fri, 10 Jul 2020 10:24:37 -0700 Subject: [PATCH 8/8] Update Sources/PenguinStructures/NumberOperations.swift --- Sources/PenguinStructures/NumberOperations.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Sources/PenguinStructures/NumberOperations.swift b/Sources/PenguinStructures/NumberOperations.swift index 5373c95a..34dd10e9 100644 --- a/Sources/PenguinStructures/NumberOperations.swift +++ b/Sources/PenguinStructures/NumberOperations.swift @@ -20,6 +20,8 @@ extension BinaryInteger { /// Returns positive integers that are coprime with, and less than, `self`. /// + /// The returned values are all unique modulo `self`. + /// /// - SeeAlso: `positiveCoprimes`. public var lesserPositiveCoprimes: [Self] { return positiveCoprimes.prefix { $0 < self }