Add ability to pin threads

[fpervaiz]

Adds a feature allowing the pool thread to be pinned to a specific core via core_affinity

[SpriteOvO]

Hi @fpervaiz!

Having the ability to set affinity for a thread in the thread pool is definitely make sense to me, the current main branch of this project lacks such ability. Thank you for coming up with this idea. 👍

Although the idea is great, I have some concerns about the current implementation of the PR.

• It can only pin one core to one thread. A thread is able to be pinned to more than one core, and there may be more than one thread in a ThreadPool in the future. Unfortunately, the new method ThreadPoolBuilder::affinity limits the possibility of such scaling. Pinning multiple cores to a thread is also impossible at the moment in crate core_affinity (see just a single cpu per thread? Elzair/core_affinity_rs#7).

• Besides affinity, there are also many other attributes of a thread, such as priority, and especially certain attributes are platform-specific. It's a bit of a burden to support these attributes on a variety of platforms, as well as for the upstream crate. Should we panic or return an error or just skip the failure if setting a attribute for a thread fails or setting on a unsupported platform? That's a tough question to answer.

I thought about it from the other side. Maybe we could provide an event hook for it, letting users do what they want after a thread is spawned and before the worker runs. The specific API still needs to be well-designed. Basically the builder will receive a closure from user, and call it at the beginning of a thread. In this way, spdlog-rs doesn't need any additional dependencies, and users can configure any attribute for a thread, even if it's not portable. But I still need a few days to figure out what the API will look like is best.

What do you think about this?

BTW, you can ignore the clippy warnings that are not related to the PR changes in CI failures, I will fix them directly on the main branch later. And thanks for you opening this PR!

[Lancern]

I thought about it from the other side. Maybe we could provide an event hook for it, letting users do what they want after a thread is spawned and before the worker runs.

I agree. This is a rather standard way to give the user a chance to glimpse and modify the new thread's environment. tokio also support this in the builder of their multi-threaded runtime: tokio::runtime::Builder::on_thread_start.

Similar to tokio, we may add a new function to ThreadPoolBuilder that registers user-defined hooks when new threads are spawned:

impl ThreadPoolBuilder {
    pub fn on_thread_start<F>(&mut self, f: F) -> &mut Self
    where
        F: Fn() + Sync + 'static,
    {
        // [...]
    }
}

[Lancern]

But for now, I think it's OK to accept this PR and do the refactor in later PRs. The changes proposed in this PR LGTM.

[Lancern]

LGTM.

[SpriteOvO]

tokio also support this in the builder of their multi-threaded runtime: tokio::runtime::Builder::on_thread_start.

@Lancern Nice reference! The signature of on_thread_start in tokio looks good to me, I think we can implement it in spdlog-rs in the same way.

Considering the concerns about the current ::affinity method, it doesn't seem like a good idea to introduce a new item that will probably be deprecated in the future. 😕 I still favor introducing on_thread_start (and on_thread_stop) in this PR straightforwardly, which also avoids unnecessary new dependencies.

@fpervaiz Would you like to continue the PR in this direction?

[fpervaiz]

Thanks for the input @Lancern @SpriteOvO. I think the on_thread_start and on_thread_stop interface is much nicer and is the way to go. I'll close this PR and open a new one for that change.