[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:32
+    auto __max_thread_count = thread::hardware_concurrency();
+    return __max_thread_count == 0 ? 1 : __max_thread_count;
+  }());
----------------
philnik wrote:
> EricWF wrote:
> > philnik wrote:
> > > EricWF wrote:
> > > > This calculation seems a bit weird to me.
> > > > 
> > > > We're basically saying that if no concurrency is reported, then we'll use 2 (not one) threads.
> > > > Why are we not just running the algorithm in the current thread in this case?
> > > > 
> > > > 
> > > > Either way, it seems like we should be using the calling thread to do some work rather than just waiting on the other threads to join.
> > > > 
> > > Frankly, I don't really care. As I already said, this is a shitty backend and shouldn't be used in production. This is more of a placeholder than anything else. We should implement a proper thread pool later and not instantiate threads every time an algorithm is called. 
> > OK, then I don't know that we should commit this.
> > 
> > What's the purpose of doing it poorly now, rather than properly later?
> > 
> The purpose is that we can check that dispatching etc. works with multiple backends and we avoid assuming too much about what the backend does, since we would otherwise run everything on a single thread.
OK, that seems like something that we don't need to check in in order for it to be initially tested.

Then, when we have a backend ready for production, we can check that in. 

Or alternatively, we can check the backend in under `test/` as a test header.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150284