[libcxx-commits] [PATCH] D150284: [libc++][PSTL] Add a simple std::thread backend

[Eric Fiselier via Phabricator via libcxx-commits]

EricWF added inline comments.


================
Comment at: libcxx/include/__algorithm/pstl_backends/cpu_backends/thread.h:38
+
+  auto __thread_count = [&]() {
+    auto __max_thread_count = __hardware_concurrency();
----------------
This doesn't need a capture default.


================
Comment at: libcxx/include/__algorithm/pstl_backends/cpu_backends/thread.h:45
+
+  const auto __brick_size = ((__last - __first) / static_cast<__diff_t>(__thread_count)) + 1;
+
----------------
This can overflow in crazy large inputs, right? 

I don't know that it's a problem, but I wanted to point it out.


================
Comment at: libcxx/include/__algorithm/pstl_backends/cpu_backends/thread.h:55
+    } else {
+      __thread_cpu_backend::__add_thread(__thread_pool, [&__f, __brick_first = __first, __brick_last = __last]() {
+        __f(__brick_first, __brick_last);
----------------
Is there somewhere in the spec it says we don't want to copy the functor? That seems potentially racy? 


================
Comment at: libcxx/include/__algorithm/pstl_backends/cpu_backends/thread.h:63
+
+  __thread_cpu_backend::__join_threads_and_destroy_pool(__thread_pool);
+}
----------------
What happens here if the user supplied function throws?


================
Comment at: libcxx/src/pstl/thread_backend.cpp:19
+
+void* __make_thread_pool(unsigned expected_thread_count) {
+  auto pool = new std::vector<thread>;
----------------
Why are we doing `void*` here?

I really really hate losing the type safety.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150284/new/

https://reviews.llvm.org/D150284