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

refactor: Reworked background helpers #100

Closed
wants to merge 7 commits into from

Conversation

adrien-coye
Copy link
Contributor

@adrien-coye adrien-coye commented Jan 31, 2024

Abstract

Rationalized the various objects available in Drive and Mail making use of ProcessInfo.performExpiringActivity(withReason). Added a high level of polish, added documentation, tests, reworked the API so on so forth.

NOTE: This is API breaking. Will be published in a 7.0.0
TODO: copy changes made here (bool should terminate) Infomaniak/ios-kDrive#1111

Copy link
Contributor Author

@adrien-coye adrien-coye left a comment

Choose a reason for hiding this comment

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

I left some notes for the reviewer.

// META keep SonarCloud happy
/// Init method
/// - Parameter qos: Optionally set a custom QoS used by underlying queues
public init(qos: DispatchQoS = .default) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can specify QoS on FlowToAsyncResult to be consistent with other core types.

public typealias TaskCompletion = () -> Void

/// Something that can perform arbitrary short background tasks, with closure API.
protocol ClosureExpiringActivable {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed BackgroundExecutor into ClosureExpiringActivity with a linked protocol ClosureExpiringActivable to move to something testable.


/// Init method of `BackgroundExecutor`
/// - Parameter qos: QoS used by the underlying queues. Defaults to `.userInitiated` to prevent most priority inversions.
public init(qos: DispatchQoS = .userInitiated) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

QoS of ClosureExpiringActivity can be specified at init.

func endAll()
}

public final class ExpiringActivity: ExpiringActivityable {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is something in Drive Core that I brought here, also cleaning it a bit, adding some tests, documenting the protocol. The whole 9 yards.

@adrien-coye adrien-coye changed the title rerfactor: Rework background helpers refactor: Reworked background helpers Feb 1, 2024
Copy link

sonarcloud bot commented Feb 1, 2024

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

#if os(macOS)
// We block a non cooperative queue that matches current QoS
DispatchQueue.global(qos: qos.qosClass).async {
self.processInfo.performActivity(options: .suddenTerminationDisabled, reason: self.id) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This call in synchronous, therefore I jump queue, unlike iOS.


#if os(macOS)
DDLogDebug("Starting task \(taskName) (No expiration handler as we are running on macOS)")
processInfo.performActivity(options: .suddenTerminationDisabled, reason: taskName) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This call been synchronous, I removed the DispatchGroup on macOS , it works out of the box.

/// - Parameters:
/// - block: The work to be performed on the background
/// - onExpired: The closure called by the system when we should end.
func executeWithBackgroundTask(_ block: @escaping (@escaping TaskCompletion) -> Void,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO: Maybe change executeWithBackgroundTask for executeWithShortBackground ?

@adrien-coye
Copy link
Contributor Author

A form of it was introduced in Transactionable PR to harden the write DB operations. Closing as most of the work was merged on top of APIV3 with Transactionable.

#121

@adrien-coye adrien-coye closed this May 3, 2024
@PhilippeWeidmann PhilippeWeidmann deleted the reworkBackgroundHelpers branch July 12, 2024 06:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant