Skip to content

Conversation

@yim-lee
Copy link
Member

@yim-lee yim-lee commented Jul 6, 2022

Resolves #958

await self.attemptToTakeForks()
self.becomeHungryTimerTask?.cancel()
break
}
Copy link
Member

Choose a reason for hiding this comment

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

Hmm okey those are just a "single" timers, so we can do these a bit simpler:

Task { 
  try await Task.sleep(until: .now + .seconds(1), clock: .continuous)
}

Copy link
Member

Choose a reason for hiding this comment

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

Note that we also don't need the cancel then -- just run it once :)

Copy link
Member Author

@yim-lee yim-lee Jul 6, 2022

Choose a reason for hiding this comment

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

amended ef06ce6

if let timerTask = self.flushTimerTasks.removeValue(forKey: timerTaskKey) {
timerTask.cancel()
}
break
Copy link
Member

Choose a reason for hiding this comment

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

That's also a single timer, so we can do

self.flushTimerTasks[timerTaskKey] = Task { 
  defer { self.flushTimerTasks.removeValue(forKey: timerTaskKey) } 
  try await Task.sleep(until: .now + flushDelay, clock: .continuous) 
}

hmm, the cancel seems un-necessary then as well; since we'll run to completion, but we do want to keep the removeValue 👍

@available(*, deprecated, message: "Actor timers cannot participate in structured cancellation, and will be replaced with patterns using swift-async-algorithms (see Timer)")
public struct TimerKey: Hashable {
// TODO: replace timers with AsyncTimerSequence from swift-async-algorithms
internal struct _TimerKey: Hashable {
Copy link
Member

Choose a reason for hiding this comment

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

great, thank you! the amount of un-necessary noise this caused was annoying

/// _BehaviorTimers are bound to this objects lifecycle, i.e. when the actor owning this object is deallocated,
/// and the `ActorTimers` are deallocated as well, all timers associated with it are cancelled.
@available(*, deprecated, message: "Actor timers cannot participate in structured cancellation, and will be replaced with patterns using swift-async-algorithms (see Timer)")
public final class ActorTimers<Act: DistributedActor> where Act.ActorSystem == ClusterSystem {
Copy link
Member

Choose a reason for hiding this comment

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

bye bye 👋 If we ever get some other type similar to this it'll be based on tasks... we'll see if it ever comes back

@yim-lee yim-lee force-pushed the async-timer-seq branch from 892a857 to ef06ce6 Compare July 6, 2022 19:00
@yim-lee yim-lee requested a review from ktoso July 6, 2022 20:56
@yim-lee
Copy link
Member Author

yim-lee commented Jul 6, 2022

@swift-server-bot test this please

self.maxSeqNr = 0
}

@discardableResult
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for the warning cleanups while here 👍

Copy link
Member

@ktoso ktoso left a comment

Choose a reason for hiding this comment

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

LGTM!

@ktoso ktoso merged commit 2f981fd into apple:main Jul 6, 2022
@yim-lee yim-lee deleted the async-timer-seq branch July 6, 2022 23:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove ActorTimers, and offer task-cancellation and async aware alternative

2 participants