Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use unspecified QoS in QueueScheduler when QoS is unspecified #880

Merged
merged 1 commit into from
Mar 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# master
*Please add new entries at the top.*

1. Change `QueueScheduler` to use unspecified QoS when QoS parameter is defaulted
1. Add support for VisionOS (#875, kudos to @NachoSoto)
1. Fix minimum deployment target of iOS 11 in CocoaPods
1. Fix CI release git tag push trigger (#869, kudos to @p4checo)
Expand Down
2 changes: 1 addition & 1 deletion Sources/Scheduler.swift
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,7 @@ public final class QueueScheduler: DateScheduler {
/// - targeting: (Optional) The queue on which this scheduler's work is
/// targeted
public convenience init(
qos: DispatchQoS = .default,
qos: DispatchQoS = .unspecified,
name: String = "org.reactivecocoa.ReactiveSwift.QueueScheduler",
targeting targetQueue: DispatchQueue? = nil
) {
Expand Down
42 changes: 42 additions & 0 deletions Tests/ReactiveSwiftTests/SchedulerSpec.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@
// Copyright (c) 2014 GitHub. All rights reserved.
//

#if canImport(Darwin)
import Darwin.sys.qos
#endif
import Dispatch
import Foundation

Expand Down Expand Up @@ -261,6 +264,45 @@ class SchedulerSpec: QuickSpec {
// enough time to ensure that the first timer was actually cancelled.
expect(count).toEventually(equal(timesToRun))
}

it("should propagate QoS values by default") {
expect(scheduler.queue.qos).to(equal(.unspecified))

// qos_class_self() may not be available on non-Darwin
// platforms, and it's unclear if QoS propagation is
// implemented in an equivalent manner in such contexts,
// so we restrict runtime validation tests to Darwin.
#if canImport(Darwin)
let userInitiatedQueue = DispatchQueue(
label: "reactiveswift.tests.user-initiated",
qos: .userInitiated
)
userInitiatedQueue.suspend()

var initialQoS: qos_class_t?
var endQoS: qos_class_t?

userInitiatedQueue.async {
initialQoS = qos_class_self()

// scheduling should propagate QoS values by default
scheduler.schedule {
endQoS = qos_class_self()
}
}

scheduler.queue.resume()
userInitiatedQueue.resume()

expect(initialQoS).toEventuallyNot(beNil())
expect(endQoS).toEventuallyNot(beNil())

expect(initialQoS).to(equal(QOS_CLASS_USER_INITIATED))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jamieQ this is failing the Linux CI job. You probably need to to add this at the top of this file:

#if canImport(Darwin)
    import Darwin
#endif

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, sorry about that (and thank you for trying to progress this). scouring the internet for a bit made me wonder if the same QoS propagation mechanisms even exist on Linux. assuming they don't, perhaps just wrapping the runtime checks here in that conditional compilation directive would be an appropriate resolution. i'll try to do a bit more research in the near term.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mluisbrown okay i think it should be good to re-run in CI at your convenience

expect(endQoS?.rawValue).to(beGreaterThanOrEqualTo(
initialQoS?.rawValue
))
#endif // canImport(Darwin)
}
}
}

Expand Down
Loading