enhance stable#432
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves stability across platforms by fixing type mismatches in Windows operator tests and making OrderedLocalQueue::is_empty() resilient to a stale-high len counter caused by cross-thread stealing.
Changes:
- Update Windows operator test token handling to cast between
usizeandu64where required by the IOCP operator API. - Change
OrderedLocalQueue::is_empty()to verify emptiness by scanning underlying per-priorityWorkers whenlen()indicates non-empty.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| core/src/net/operator/windows/tests.rs | Align token types with the Windows operator’s u64 user-data/token interface via explicit casts. |
| core/src/common/ordered_work_steal.rs | Make local-queue emptiness checks robust to stale-high counters by scanning workers when needed. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| //把本地队列的一半放到全局队列 | ||
| let count = self.len() / 2; | ||
| let count = self.local_len() / 2; | ||
| for _ in 0..count { | ||
| for entry in self.queue.iter().rev() { | ||
| if let Some(item) = entry.value().pop() { | ||
| self.shared.push_with_priority(*entry.key(), item); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
push_to_global subtracts count from the local counter, but the nested loops can pop more (or fewer) than count items because each outer iteration can pop up to one item per priority entry. This will desynchronize local_len() from the actual number of local items. Track the exact number of items actually moved to the shared queue and adjust the local counter by that value (or recompute local occupancy after the transfer).
| // refresh local len | ||
| self.len.store( | ||
| self.local_len().saturating_add( | ||
| into_queue.capacity() - into_queue.spare_capacity(), | ||
| ), | ||
| Ordering::Release, | ||
| ); |
There was a problem hiding this comment.
After a successful sibling steal, the “refresh local len” logic adds into_queue.capacity() - into_queue.spare_capacity() to the existing local_len(). This value is the total current size of into_queue, not the number of newly stolen items, so it can overcount (especially when the priority bucket already had items). Consider capturing the stolen count inside the steal closure or recomputing the full local occupancy across all buckets and storing that.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #432 +/- ##
==========================================
- Coverage 75.46% 75.45% -0.01%
==========================================
Files 44 44
Lines 4675 4698 +23
==========================================
+ Hits 3528 3545 +17
- Misses 1147 1153 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Add CoroutinePool::is_local_empty() method and use it in event loop thread exit condition and do_stop_sync() shutdown check. After PR #432, is_empty() checks all queues globally (including siblings), causing the event loop to block until all sibling queues drain. Using is_local_empty() restores local-only exit semantics so each event loop exits when its own work is done. Agent-Logs-Url: https://github.com/acl-dev/open-coroutine/sessions/a25cbffc-7ce7-4db8-a618-776a75374e56 Co-authored-by: loongs-zhang <38336731+loongs-zhang@users.noreply.github.com>
// Describe your PR here; eg. Fixes #issueNo
Make sure that: