bkietz commented on pull request #8582: URL: https://github.com/apache/arrow/pull/8582#issuecomment-724072730
> I'm roughly ok with making ThreadPool private, but why is it necessary to do so? Currently `thread_pool.h` includes `future.h` because `Executor::Submit` depends on a complete definition of Future (and since these are templates we can't just move Submit's definition to a source file). This means that in the definition of Future `Executor` must be left incomplete. My goal was to allow use of the `Executor` interface to specify where continuations should be executed (analagously to libprocess' defer-to-PID mechanic). I moved Executor into the same header file to allow usage of Executor within Future and of Future within Executor. Another way to accomplish this would be requiring that only FutureImpl (or other non-templates) may use Executor and write code handling deferral of continuations in `future.cc`, but it seemed preferable to make ThreadPool private and ensure that the Executor interface was sufficiently useful that no code needs to downcast to ThreadPool. This may be overly strict OO-ification, but: if ThreadPool is the only implementation of the Executor interface and some code requires a pointer to the concrete class while for other code a pointer to the interface suffices then the interface should either be extended to provide the extra methods or removed for simplicity. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected]
