Skip to content

Commit 2bc5623

Browse files
LucianoPAlmeidanatecook1000
authored andcommitted
[stdlib] Resolving some FIXME comments on Set type. (#20631)
* Fixing some fixmes on stdlib Set * Adding @inline attr * Fixing spaces * Adding isEmpty as fast path in other places where is possible. * Quotes on variable name on comment. * Update stdlib/public/core/Set.swift Co-Authored-By: LucianoPAlmeida <[email protected]> * Adding benchmark for isDisjoint Set method * Adding empty sets to benchmark * Fixing the factor on benchmarks and naming warnings for empty 5 words
1 parent 5c7fd17 commit 2bc5623

File tree

2 files changed

+107
-10
lines changed

2 files changed

+107
-10
lines changed

benchmark/single-source/SetTests.swift

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@ let size = 400
1616
let half = size / 2
1717
let quarter = size / 4
1818

19+
// Construction of empty sets.
20+
let setE: Set<Int> = []
21+
let setOE: Set<Box<Int>> = []
22+
1923
// Construction kit for sets with 25% overlap
2024
let setAB = Set(0 ..< size) // 0 ..< 400
2125
let setCD = Set(size ..< 2 * size) // 400 ..< 800
@@ -53,6 +57,11 @@ let setQ: Set<Int> = {
5357

5458
public let SetTests = [
5559
// Mnemonic: number after name is percentage of common elements in input sets.
60+
BenchmarkInfo(
61+
name: "Set.Empty.IsSubsetInt0",
62+
runFunction: { n in run_SetIsSubsetInt(setE, setAB, true, 5000 * n) },
63+
tags: [.validation, .api, .Set],
64+
setUpFunction: { blackHole([setE, setAB]) }),
5665
BenchmarkInfo(
5766
name: "SetIsSubsetInt0",
5867
runFunction: { n in run_SetIsSubsetInt(setAB, setCD, false, 5000 * n) },
@@ -84,6 +93,47 @@ public let SetTests = [
8493
tags: [.validation, .api, .Set],
8594
setUpFunction: { blackHole([setP, setQ]) }),
8695

96+
BenchmarkInfo(
97+
name: "Set.Empty.IsDisjointInt0",
98+
runFunction: { n in run_SetIsDisjointInt(setE, setAB, true, 50 * n) },
99+
tags: [.validation, .api, .Set],
100+
setUpFunction: { blackHole([setE, setAB]) }),
101+
BenchmarkInfo(
102+
name: "Set.Empty.IsDisjointBox0",
103+
runFunction: { n in run_SetIsDisjointBox(setOE, setOAB, true, 50 * n) },
104+
tags: [.validation, .api, .Set],
105+
setUpFunction: { blackHole([setOE, setOAB]) }),
106+
BenchmarkInfo(
107+
name: "SetIsDisjointInt0",
108+
runFunction: { n in run_SetIsDisjointInt(setAB, setCD, true, 50 * n) },
109+
tags: [.validation, .api, .Set],
110+
setUpFunction: { blackHole([setAB, setCD]) }),
111+
BenchmarkInfo(
112+
name: "SetIsDisjointBox0",
113+
runFunction: { n in run_SetIsDisjointBox(setOAB, setOCD, true, 50 * n) },
114+
tags: [.validation, .api, .Set],
115+
setUpFunction: { blackHole([setOAB, setOCD]) }),
116+
BenchmarkInfo(
117+
name: "SetIsDisjointInt25",
118+
runFunction: { n in run_SetIsDisjointInt(setB, setAB, false, 50 * n) },
119+
tags: [.validation, .api, .Set],
120+
setUpFunction: { blackHole([setB, setAB]) }),
121+
BenchmarkInfo(
122+
name: "SetIsDisjointBox25",
123+
runFunction: { n in run_SetIsDisjointBox(setOB, setOAB, false, 50 * n) },
124+
tags: [.validation, .api, .Set],
125+
setUpFunction: { blackHole([setOB, setOAB]) }),
126+
BenchmarkInfo(
127+
name: "SetIsDisjointInt50",
128+
runFunction: { n in run_SetIsDisjointInt(setY, setXY, false, 50 * n) },
129+
tags: [.validation, .api, .Set],
130+
setUpFunction: { blackHole([setY, setXY]) }),
131+
BenchmarkInfo(
132+
name: "SetIsDisjointInt100",
133+
runFunction: { n in run_SetIsDisjointInt(setP, setQ, false, 50 * n) },
134+
tags: [.validation, .api, .Set],
135+
setUpFunction: { blackHole([setP, setQ]) }),
136+
87137
BenchmarkInfo(
88138
name: "SetSymmetricDifferenceInt0",
89139
runFunction: { n in run_SetSymmetricDifferenceInt(setAB, setCD, countABCD, 10 * n) },
@@ -177,6 +227,16 @@ public let SetTests = [
177227
tags: [.validation, .api, .Set],
178228
setUpFunction: { blackHole([setP, setQ]) }),
179229

230+
BenchmarkInfo(
231+
name: "Set.Empty.SubtractingInt0",
232+
runFunction: { n in run_SetSubtractingInt(setE, setAB, 0, 10 * n) },
233+
tags: [.validation, .api, .Set],
234+
setUpFunction: { blackHole([setE, setAB]) }),
235+
BenchmarkInfo(
236+
name: "Set.Empty.SubtractingBox0",
237+
runFunction: { n in run_SetSubtractingBox(setOE, setOAB, 0, 10 * n) },
238+
tags: [.validation, .api, .Set],
239+
setUpFunction: { blackHole([setOE, setOAB]) }),
180240
BenchmarkInfo(
181241
name: "SetSubtractingInt0",
182242
runFunction: { n in run_SetSubtractingInt(setAB, setCD, countAB, 10 * n) },
@@ -296,6 +356,18 @@ public func run_SetSubtractingInt(
296356
}
297357
}
298358

359+
@inline(never)
360+
public func run_SetIsDisjointInt(
361+
_ a: Set<Int>,
362+
_ b: Set<Int>,
363+
_ r: Bool,
364+
_ n: Int) {
365+
for _ in 0 ..< n {
366+
let isDisjoint = a.isDisjoint(with: identity(b))
367+
CheckResults(isDisjoint == r)
368+
}
369+
}
370+
299371
class Box<T : Hashable> : Hashable {
300372
var value: T
301373

@@ -371,3 +443,15 @@ func run_SetSubtractingBox(
371443
CheckResults(and.count == r)
372444
}
373445
}
446+
447+
@inline(never)
448+
func run_SetIsDisjointBox(
449+
_ a: Set<Box<Int>>,
450+
_ b: Set<Box<Int>>,
451+
_ r: Bool,
452+
_ n: Int) {
453+
for _ in 0 ..< n {
454+
let isDisjoint = a.isDisjoint(with: identity(b))
455+
CheckResults(isDisjoint == r)
456+
}
457+
}

stdlib/public/core/Set.swift

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -712,7 +712,8 @@ extension Set: SetAlgebra {
712712
@inlinable
713713
public func isSubset<S: Sequence>(of possibleSuperset: S) -> Bool
714714
where S.Element == Element {
715-
// FIXME(performance): isEmpty fast path, here and elsewhere.
715+
guard !isEmpty else { return true }
716+
716717
let other = Set(possibleSuperset)
717718
return isSubset(of: other)
718719
}
@@ -763,10 +764,12 @@ extension Set: SetAlgebra {
763764
@inlinable
764765
public func isSuperset<S: Sequence>(of possibleSubset: __owned S) -> Bool
765766
where S.Element == Element {
766-
// FIXME(performance): Don't build a set; just ask if every element is in
767-
// `self`.
768-
let other = Set(possibleSubset)
769-
return other.isSubset(of: self)
767+
for member in possibleSubset {
768+
if !contains(member) {
769+
return false
770+
}
771+
}
772+
return true
770773
}
771774

772775
/// Returns a Boolean value that indicates whether the set is a strict
@@ -811,9 +814,7 @@ extension Set: SetAlgebra {
811814
@inlinable
812815
public func isDisjoint<S: Sequence>(with other: S) -> Bool
813816
where S.Element == Element {
814-
// FIXME(performance): Don't need to build a set.
815-
let otherSet = Set(other)
816-
return isDisjoint(with: otherSet)
817+
return _isDisjoint(with: other)
817818
}
818819

819820
/// Returns a new set with the elements of both this set and the given
@@ -920,6 +921,10 @@ extension Set: SetAlgebra {
920921
@inlinable
921922
internal mutating func _subtract<S: Sequence>(_ other: S)
922923
where S.Element == Element {
924+
// If self is empty we don't need to iterate over `other` because there's
925+
// nothing to remove on self.
926+
guard !isEmpty else { return }
927+
923928
for item in other {
924929
remove(item)
925930
}
@@ -1121,8 +1126,16 @@ extension Set {
11211126
/// otherwise, `false`.
11221127
@inlinable
11231128
public func isDisjoint(with other: Set<Element>) -> Bool {
1124-
for member in self {
1125-
if other.contains(member) {
1129+
return _isDisjoint(with: other)
1130+
}
1131+
1132+
@inlinable
1133+
internal func _isDisjoint<S: Sequence>(with other: S) -> Bool
1134+
where S.Element == Element {
1135+
guard !isEmpty else { return true }
1136+
1137+
for member in other {
1138+
if contains(member) {
11261139
return false
11271140
}
11281141
}

0 commit comments

Comments
 (0)