I'm curious what the new samply record graphs look like after this change.

I'm not a fan of relying on Drop for this to be safe. I would rather this be modeled like scoped threads where the lifetimes associated with the scope prevent out-living the benchmarked closure's lifetime.

The effects on samply record target/release/deps/threads-XXX --bench arc look like this, which looks very nice indeed, and shows clearly that we only ever create a total of 16 threads.

before

after

I also agree that the current solution is neither very sound, nor nice. I’m tempted to just copy-paste the whole std::thread::scope code and move it into a crate that reuses threads across invocations.
A cursory look at crates.io shows that there are a couple of "scoped thread pool" crates, but neither looks quite like what I would like to see.

I’m tempted to just copy-paste the whole std::thread::scope code and move it into a crate that reuses threads across invocations.

I was playing around with that a little, but std::thread::scope uses some private internals of std, so that didn’t work out. Then I tried using a crossbeam_utils as a starting point but gave up halfway through.

I would rather this be modeled like scoped threads where the lifetimes associated with the scope prevent out-living the benchmarked closure's lifetime.

I agree. I tried to copy-paste at least that part from the std implementation, together with a bunch of PhantomData similar to how it is used there, but I also gave up on fighting the borrow checker there. Maybe I will return to that at some point, but I’m a bit too lazy right now 😅