-
Notifications
You must be signed in to change notification settings - Fork 7
@MainActor annotations #42
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
Conversation
lickel
commented
Oct 13, 2022
- Annotate many methods as @mainactor
- All delegate methods
- All code with assert(Thread.isMainThread)
- Faulting an association when you're off the main thread will have different characteristics
- If the association already exists, nothing will change
- If the association does not already exit, it will always return nil and hit the main thread to batch fetch the associations
Codecov ReportBase: 93.90% // Head: 93.44% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## feature/5.0 #42 +/- ##
===============================================
- Coverage 93.90% 93.44% -0.46%
===============================================
Files 30 30
Lines 4958 5038 +80
===============================================
+ Hits 4656 4708 +52
- Misses 302 330 +28
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
| SKIP_INSTALL = YES; | ||
| SWIFT_ACTIVE_COMPILATION_CONDITIONS = DEBUG; | ||
| SWIFT_OPTIMIZATION_LEVEL = "-Onone"; | ||
| SWIFT_STRICT_CONCURRENCY = targeted; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This runs "clean".
Full concurrency checking would fail in many ways (notably @MainActor(unsafe) does not work).
|
|
||
| internal class FetchRequestObservableToken<Parameter>: ObservableToken { | ||
| private let _observe: (_ handler: @escaping (Parameter) -> Void) -> Void | ||
| typealias Handler = (Parameter) -> Void |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Making this @mainactor (which requires making ObservableToken's handler @mainactor) was a giant pain and not worth it since I could do the simple main-thread bouncing behavior.
|
|
||
| guard let index = fetchedObjects.firstIndex(where: { $0.id == objectID }) else { | ||
| if !Thread.isMainThread { | ||
| DispatchQueue.main.async { [weak self] in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
performOnMainThread would need to support throwing in an awkward way if I wanted to reuse it here.
|
|
||
| let handleSort: (FetchedObject, Bool, Any?, Any?) -> Void = { [weak self] object, isSection, old, new in | ||
| let handleSort: @MainActor (FetchedObject, Bool, Any?, Any?) -> Void = { [weak self] object, isSection, old, new in | ||
| assert(Thread.isMainThread) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This always required main thread behavior
|
|
||
| let handleChange: (FetchedObject) -> Void = { [weak self] object in | ||
| let handleChange: @MainActor (FetchedObject) -> Void = { [weak self] object in | ||
| assert(Thread.isMainThread) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This always required main thread behavior
|
|
||
| memoryPressureToken?.observeIfNeeded { [weak self] notification in | ||
| self?.removeAllAssociatedValues() | ||
| performOnMainThread { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New behavior: support for bouncing to main for these handful of observations
CHANGELOG.md
Outdated
| * All code with assert(Thread.isMainThread) | ||
| * Faulting an association when you're off the main thread will have different characteristics | ||
| * If the association already exists, nothing will change | ||
| * If the association does not already exit, it will always return nil and hit the main thread to batch fetch the associations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
exist and not exit here, I think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hah! yup, fixed