compnerd added inline comments.
================ Comment at: include/__threading_support:300-305 +int __libcpp_recursive_mutex_init(__libcpp_mutex_t *__m) +{ + InitializeSRWLock(__m); + return 0; +} + ---------------- majnemer wrote: > I don't think you can use slim rw locks for recursive locks. I think we will > need to use `CRITICAL_SECTION` for those. std::recursive_mutex can't be used > with std::condition_variable AFAIK so all you need (I think) is recursive > versions of `__libcpp_mutex_...` > > Recursive locks should be used far less frequently which makes it valuable, > IMO, to use slim rw locks for the non-recursive mutex implementation. You are absolutely right. That was something that I looked at originally and went with the CS. However, the overhead of a tagged struct is 5 or 9 bytes (sizeof(void *) + 1) bytes (ignoring padding for MS ABI). Going with that should give the benefits of always being able to properly initialize the CS instead of the kludge. ================ Comment at: include/__threading_support:355 + // TODO(compnerd) handle spurious timeout + if (!SleepConditionVariableSRW(__cv, __m, + duration_cast<milliseconds>(timeout).count(), ---------------- majnemer wrote: > I don't think it should be `__libcpp_condvar_timedwait'`s problem. > `__libcpp_condvar_timedwait` wraps `pthread_cond_timedwait` on POSIX > platforms and the caller of `__libcpp_condvar_wait` properly handles spurious > wakeups. The caller of `__libcpp_condvar_timedwait` probably should be > audited. SG; seems that there is a single user in condition_variable.cpp Repository: rL LLVM https://reviews.llvm.org/D28220 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits