Finish moving package collections to async/await #7746
Conversation
AndrewHoos
requested review from
bnbarham,
MaxDesiatov,
jakepetroules,
francescomikulis and
xedin
as code owners
|
@swift-ci test |
There was a problem hiding this comment.
IMO the use of DispatchQueue.sharedConcurrent in this context can lead to thread explosion, especially as we still have temp_await in different places in our codebase. Apparently, Dispatch will spawn more threads if any of the existing threads are stalled on a semaphore, which is what temp_await does.
AndrewHoos
force-pushed
the
PackageCollectionsStorage
branch
from
e8eb0a9 to
7c6dcfc
Compare
|
@swift-ci test |
AndrewHoos
force-pushed
the
PackageCollectionsStorage
branch
2 times, most recently
from
45a01ed to
74afd50
Compare
|
@swift-ci test |
|
@swift-ci test windows |
AndrewHoos
force-pushed
the
PackageCollectionsStorage
branch
from
74afd50 to
1c5ae6b
Compare
|
@swift-ci test |
|
@swift-ci test windows |
| @@ -48,63 +34,51 @@ struct FilePackageCollectionsSourcesStorage: PackageCollectionsSourcesStorage { | |||
| } | |||
|
|
|||
| func list() async throws -> [PackageCollectionsModel.CollectionSource] { | |||
There was a problem hiding this comment.
There are no async calls in this and other methods that would require await, maybe async effect and locking from it should be removed, while the type itself made an actor to synchronize access?
There was a problem hiding this comment.
I have two concerns with this.
1- It looks like motivating reason for the callbacks was parallel decoding of large collections. Maybe that is nontrivial necessary and was premature optimization.
2- This could be made an actor, but it does not solve synchronization between processes. If more than one process is accessing the same storage on disk then the synchronization needs to be done with file locks not swift concurrency isolation
There was a problem hiding this comment.
Ah, I missed those were file locks, thanks for the clarification
Finish moving package collections to async/await
Motivation:
Finish the replacement of callback/result APIs with async/await
Modifications:
Move SQL queries off of the Swift concurrency queue
Move PackageCollectionsStorage protocol to async/await
Result:
No more callback based APIs in package collections