Enhance Job Queue Implementation with Priority-Based Reordering and SwiftDocs Annotations by admirsaheta · Pull Request #276 · swiftwasm/JavaScriptKit
I have a few comments:
-
Foundation dependency:
We aim to avoid depending on Foundation in this library, as it tends to increase binary size significantly. -
Queue priority support?:
You saidRefactored the queue to support job insertion and execution based on priority.
However it seems the current implementation already supports job insertion and execution based on priority, though the header comment might be misleading.
-
Concurrent access lock:
Introduced a queueLock to handle concurrent job insertion safely.
Sorry, I think the implementation does not protect anything as
queueLockcreates a new NSLock instance everytime. Could you please test concurrent access scenarios?
Finally, as this module is a critical part of the concurrency support in WebAssembly, we generally prefer to make changes only if they provide clear benefits, such as fixing bugs or improving performance. For other types of changes, we may be less likely to merge them, but we truly appreciate your contribution and effort.