This is a nice improvement, but IMO spin locks should be just that: basic spinning locks (unless there's a compelling reason against that). They can be useful in test code to minimize latency.
Perhaps we should remove all uses of spin locks from winpthreads? Best Regards, Luca ________________________________ Da: Kirill Makurin <[email protected]> Inviato: giovedì 21 maggio 2026 07:53 A: [email protected] <[email protected]> Oggetto: Re: [Mingw-w64-public] winpthreads: fix XFAIL tests and provide new spinlock implementation I wrote: > I guess winpthreads' `pthread_spinlock_t` could be a mutex in disguise or > something like that, in which case first implementation could be more > flexible in the sense that we can change underlying structure and logic > anytime we want. I can as well update first implementation to not use loops > at all and instead use some Windows object for synchronization. So, here's another implementation for spinlocks. This time, it uses auto-reset events; it is mostly original 0004.txt updated to use auto-reset events instead of loops. Unlike second version, no other changes to original patch series needed. I pushed this version to run through CI[1] and it passes. If we are to go with this event-based implementation, we need to keep in mind that winpthreads uses `pthread_spinlock_t` internally: - one global spinlock in barrier.c - one global spinlock in cond.c; used by pthread_cond_destroy and when using statically initialized `pthread_cond_t` object for the first time - two global spinlocks in rwlock.c - one spinlock per thread created with pthread_create What this means is that with event-based implementation winpthreads will use more system resources. I'm not saying that this is a problem, I'm just pointing it out. Another thing to mention is that the way barrier.c and rwlock.c use spinlocks is ridiculous... they lock and unlock global (!) spinlock whenever they increment/decrement reference count in *any* `pthread_rwlock_t`/`pthread_barrier_t` object. I feel like this is an overkill and it could be done better, but this is for another time. While testing this version locally, I noticed that some tests, or to be specific rwlock7 and rwlock8, take significantly more time to complete. Those tests print total amount of time test was running, so I started comparing... (Note that I took numbers from random `make check` runs. Obviously, they will differ from one run to another, so treat them as an approximation.) Tests rwlock7 and rwlock8 are nearly the same; there are two differences: 1. rwlock8 calls `sched_yield` before calling `pthread_rwlock_unlock` 2. rwlock8 does 100,000 iterations per thread, while rwlock7 does 1,000,000 (5 threads total in each test) With current spinlock implementation, they take 1,150ms and 226ms respectively. With implementation I sent last week, they take 1,560ms and 268ms respectively. With new event-based implementation, they take ridiculous 138,142ms and 13,810ms respectively. It does not mean that new event-based implementation is slow; as I mentioned, use of spinlocks in rwlock.c is ridiculous (at least in my opinion). What matters is that those results were when I ran `make check` sequentially, that is, without -j. When I was running `make check -j`, I saw that in some cases rwlock8 (100,000 iterations) would take the same or even more time to complete than rwlock7 (1,000,000 iterations). Meanwhile, results with event-based implementation were fairly consistent; the longest time I saw was about 150,000ms and 15,000ms respectively. We can go with this event-based implementation, in which case no changes to previously sent patches needed. Or, we can go with simplistic implementation I sent last week, in which case I'll need to update some follow up changes in tests. - Kirill Makurin [1] https://github.com/maiddaisuki/mingw-w64/actions/runs/26203279301 _______________________________________________ Mingw-w64-public mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/mingw-w64-public
