Minor improvements to nonblocking synchronization. #2272
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #2272 +/- ##
=======================================
Coverage 72.05% 72.05%
=======================================
Files 155 155
Lines 14770 14776 +6
=======================================
+ Hits 10642 10647 +5
- Misses 4128 4129 +1 ☔ View full report in Codecov by Sentry. |
| # should be safe to assign before threads are running; | ||
| # any user will just submit work that makes it block | ||
| lock(sync_channel_lock) do | ||
| # test and test-and-set |
There was a problem hiding this comment.
TATAS doesn't look safe here to me, since BidirectionalChannel is a struct, not a pointer, so it doesn't have atomic updates in an array. Maybe make it mutable for now? This is the reason for JuliaLang/julia#51495, but we don't have that merged yet. It could also now be an AtomicMemory object, but (a) that is only in v1.11 (b) it doesn't have the public API written yet (c) would incur a lock allocation for every element and every read or write access, which seems undesired here
|
|
||
| # we don't know what the size of uv_thread_t is, so reserve enough space | ||
| tid = Ref{NTuple{32, UInt8}}(ntuple(i -> 0, 32)) | ||
| # we don't know what the size of uv_thread_t is, so reserve enough space |
There was a problem hiding this comment.
FWIW, uv_thead_t is a Csize_t AFAIK, and requires alignment as such, which this does not have
I noticed the call to
uv_detachand use ofBase.uv_errorin JuliaLang/julia#53422, and it also seems better to put a lock around the write tosync_channelsto avoid unnecessary threads being created.