[PATCH] D72301: [ORC] Fix the move-captured std::unique_ptr vs. std::function dilemma

[Lang Hames via Phabricator via llvm-commits]

lhames added a comment.

> The current workaround creates and copies a std::shared_pointer. That's quite expensive. In a first step we can move-capture it in the Work lambda to avoid the copy. I think we don't get around the creation (assuming we keep a smart pointer), so why not receive it as a std::shared_ptr directly? The only disproportionate overhead I see is in the materializeOnCurrentThread case. I changed that to not calling the function at all.

In the context of running a Materializer I don’t think we need to be worried about the shared_ptr construction overhead, but it would still be nice to tidy this up.

For our purposes it would be ideal if ThreadPool::async took an llvm::unique_function rather than a std::function. I wonder if that would work for other clients too? I think 99% of ThreadPool use cases would work equally well or better with a unique_function, and it looks like unique_function should be constructible from a std::function (with minor runtime overhead for execution) for the 1% of cases that are not supported.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72301