Open carllerche opened 9 years ago
This should be easy to do once we drop stdout
and stderr
.
Triage: no change.
I don't see stdout and stderr inside thread::Builder
itself, so it seems like we could do this now with little difficulty, though I may be missing some detail.
stdout
and stderr
methods have gone away since d4a2c94. Changing self
to &self
in spawn
will not break any existing code(I think).
Once concern is that we need the String
name contained by-value, not a reference to it, but that feels like a relatively minor concern to me. We can definitely implement Clone for Builder, though, I think, as the first (and perhaps only) step here. @rust-lang/libs Can we get pre-approval for implementing Clone for the thread Builder? If so, then I can mentor/E-easy. I've included the builder struct below (in source it's here).
pub struct Builder {
// A name for the thread-to-be, for identification in panic messages
name: Option<String>,
// The size of the stack for the spawned thread in bytes
stack_size: Option<usize>,
}
The one difficulty with Clone
that I could think of is that you typically can't clone trait objects (e.g. closures). This is why, for example, Command
does not implement Clone
. We may be able to get around this though perhaps, as I've certainly found cloning builders historically to be useful.
I'm somewhat confused -- Builder doesn't store a closure, does it? So cloning it shouldn't be a problem...?
Builder
only specifies the thread name and the stack size. Thread body is given when spawn
is called, so cloning the Builder
shouldn't be a problem.
Ah yes sorry to clarify I mean for future expansion of builder methods. At Rust 1.0.0 the Command
type did not contain any closures but it's since picked some up. If we add Clone
here we'd just want to be sure that we'd never want to add some sort of callback to the builder.
Then it'd be more desireable to add a reusable variant of Builder
, or adding a method that clones the Builder
without implementing Clone
. Maybe we can add borrowing variant of spawn
that calls callback if there's any, and then invalidates it, I think?
Suggestion that should be forwards compatible: a function that manually clones the builder, then calls that new builder's spawn. A single function can be deprecated and implementation adjusted.
It'd be interesting to see a concrete example where having a re-usable thread builder would be useful. It doesn't actually carry much state itself. Otherwise, since we don't tend to have re-usable builders in the standard library we can probably close this.
My use case was to be able to spawn multiple threads with identical configurations and names. Atm, you have to wrap the builder with a function that creates the builder and calls spawn. Any library that spawns multiple threads automatically, including every async runtime that supports multiple threads, can use this.
Builders are good for two things: either branching logic in the construction process or re-usability. Given the limited data that is being used in the builder and the fact that it's not reusable atm, it's quite useless.
Consider making
std::thread::Builder
reusable or have a reusable variant. This would be a helpful construct to use as part of a thread pool or any other abstraction that spawns threads.