5

Does it make sense to use std::shared_ptr < std::thread> ? The logic is simple: if thread is not needed, delete it, if new is required - realocate it. is there any way to compare this concept with pooled threads?

I do know the exact ammount of threads in my system(I develop Image processing algorithm, and I wanted to give each child of "algorithm" class an individual thread (maybe to make it private, then no shared_ptr is required), where this algorithm will be running, and idle this private thread if no image was provided. Is it a bad concept?

hagor
  • 304
  • 1
  • 15
  • 5
    IMHO: don't use a pointer unless you absolutely need to. – NathanOliver May 15 '18 at 14:35
  • Perhaps https://stackoverflow.com/questions/9127816/stdshared-ptr-thread-safety-explained ? – Severin Pappadeux May 15 '18 at 14:37
  • 3
    Shared ownership of a `thread` makes little sense. Whatever is going to `join()` it should also own it. – Igor Tandetnik May 15 '18 at 14:38
  • i think a detached thread would be better – Tyker May 15 '18 at 14:42
  • do you plan to detach? If yes then sure - I can sort of see the value; if not; you're just making a nice easy way to have std::terminate called – UKMonkey May 15 '18 at 14:42
  • "Private thread" sounds like a bad idea to me. IMO, the code that manages "worker threads" should be independent of the code that defines the work that the threads do. In other words, I'm saying, use a _[thread pool](https://en.wikipedia.org/wiki/Thread_pool)_. – Solomon Slow May 15 '18 at 15:22
  • 1
    @tyker `detach()` is not normally recommended because process termination is brutal and rare and non-reproducible errors might be encountered because they're doing something critical when they're brutally culled. It's a classic bug that doesn't show up in relatively low load free running tests and then fouls in high load production just when you don't want it and have a real job debugging and fixing it. Essentially every detach() that isn't synchronized back is a race condition. We hate those. – Persixty May 15 '18 at 16:11
  • @jameslarge I'd fight shy of 'bad idea'. Though thread-pool is probably better. Thread-pool really helps if you have two tasks of type X and the private thread model will do them in sequence when they could be parallel in separate threads. Remember thread-pools have an overhead of enqueuing and dequeuing tasks. There are no free lunches. I think I want to reserve 'bad idea' for 'won't work' not just likely sub-optimal. – Persixty May 15 '18 at 16:17
  • @Persixty, Overhead? of `q.put(...)` or `q.take()`? Really? I would expect that to be utterly insignificant compared to the cost of performing any task that's worth handing off to a worker thread. – Solomon Slow May 15 '18 at 16:32
  • @Persixty, A thread "pool" that has just one worker thread will perform tasks in the order that they were given if that's what you want. My point was, that [separation of concerns](https://effectivesoftwaredesign.com/2012/02/05/separation-of-concerns/) usually is a good idea. A thread pool separates the responsibility for managing the life cycle of threads from whatever it is that the given tasks are responsible to do. – Solomon Slow May 15 '18 at 16:36
  • 1
    @jameslarge I think I see what you're getting at. The worker could well be some class other than `std::thread<>` and that does offer options like cleanly evolving into from 1 to multiple threads in a future version. Say V1: processes colour image to grey scale in 1. V2: Cuts it up and does different pieces in parallel. That said one advantage of close coupling may be to achieve 25fps reliably. While separation is a good aim, it's still a matter of which lunch you want to pay for. All non-trivial real-time DIP application will be performance challenged at some point and ultimately bound by it. – Persixty May 15 '18 at 16:51

2 Answers2

12

You probably miss the fact fact std::thread destructor does not terminate the thread. As already mentioned in the comments, if detach or join was not called before, std::thread destructor calls std::terminate. In other words, std::shared_ptr<std::thread> is pretty useless.

A std::thread is a rather low-level object. You may like to have a look at:

Maxim Egorushkin
  • 131,725
  • 17
  • 180
  • 271
  • 4
    You could provide a custom deleter to `join` with the thread before closing it. Though I don't think that's a good design in general, the few cases that `std::shared_ptr` might be useful also seem like the cases where such a deleter could work. – François Andrieux May 15 '18 at 15:04
  • 1
    @FrançoisAndrieux Well, calling `std::thread::join` does not terminate the thread either. – Maxim Egorushkin May 15 '18 at 15:06
  • So as far as I understand, it is better to give the private thread for each object inside the system, but not to make it shared? And the best way is to make it pooled, is it? So shall I dynamicly change the number of threads in pool in case I connect one more camera? – hagor May 15 '18 at 15:07
  • 1
    @hagor It depends on what you intend to use those threads for. There are multiple patterns of how threads can be used. – Maxim Egorushkin May 15 '18 at 15:10
  • @MaximEgorushkin You're right, I misunderstood the context. – François Andrieux May 15 '18 at 15:12
  • @hagor For parallel computations you make like to have a look at "parallel stl". – Maxim Egorushkin May 15 '18 at 15:14
2

This answer is in the question:

give each child of "algorithm" class an individual thread (maybe to make it private, then no shared_ptr is required)

Only use std::shared_ptr<> where ownership is genuinely shared.

There's usually no harm in idle threads hanging about but on many systems there is an overhead of even a ceiling on thread instances even if many are not running.

If there's a risk of thread proliferation or too much creating and destroying of threads and swapping, introduce a thread-pool and still don't use shared_ptr<> because the pool with own the threads.

Persixty
  • 8,165
  • 2
  • 13
  • 35
  • 1
    I disagree that there is no harm in leaking thread objects. Proper designed should not leak resources. – François Andrieux May 15 '18 at 15:06
  • Thank you, for reply! I do image processing with let's say 25 frames pro second, and if my algorithm is fast enough, my algorithm thread is hanging for lets say (1/50 second) that is a leak of resources. In other case, let's say, my algorithm is a bit slower then fps, so I will need one more thread, which will be taken from the pool. Which is a good design. The question is: shall i somehow change the number of threads inside the pool? – hagor May 15 '18 at 15:11
  • Re, "Only use `shared_ptr<>` where ownership is genuinely shared." I've worked on projects where we decided that _all_ pointers should be `shared_ptr`, regardless of whether the ownership actually was shared or not. The reason was, simplicity: Only one kind of pointer semantics to think about. – Solomon Slow May 15 '18 at 15:18
  • 1
    First answer first. In real-time image processing you should try and avoid changing the pool size after setup. A reason for using a pool is that creating and terminating threads has an overhead such as allocating a stack for it to use. That might show up as lag in your animation. If you allocate too many there'll be lots of task swapping overhead. Allocate too few you're not taking full advantage. use http://en.cppreference.com/w/cpp/thread/thread/hardware_concurrency to determine what is there. You should probably use less than that as your pool is not the only process. You'll end up tuning. – Persixty May 15 '18 at 15:29
  • 1
    Second answer, 'only' use `shared_ptr` isn't unreasonable but general advice is not to. `unique_ptr` is practically overhead free in execution and shared has an overhead. But worse you can't (easily) take ownership back from `shared_ptr` but `unique_ptr;;release()` gives it back. That can be important for inter-operability with say library code that expects ownership. I'd say both are so ubiquitous in modern C++ that mix-and-match shouldn't really confound anyone. In fact 'object x owns the object' is the commonest case and has the simplest semantics.... – Persixty May 15 '18 at 15:38
  • .. Indeed it often doesn't make sense for some contained object to survive its owner and `shared_ptr` might invite errors in thinking it can. – Persixty May 15 '18 at 15:41
  • @Persixty, Thank you! I will continue reading. Shall I give the maximum job number for my joblist? – hagor May 15 '18 at 15:44
  • @FrançoisAndrieux I'm not advocating leaking but (say) some class of object has a private worker thread for each instance that can work. Look at services in an operating system - possibly hundreds of processes/threads most of which are mostly idle but a good way to ensure resilence. That said, a thread-pool is likely a smarter answer. PS: It's really bad style to just let threads leak and never get joined (e.g. in the owner case in the destructor). I see too many examples that fall out of `main()` without `join()` - much worse than not calling `delete` because it hids 'dead thread' bugs. – Persixty May 15 '18 at 15:52
  • @hagor Not sure what you mean. It's not always possible to limit the queue length because the options are wait to add (pointless) or fail. In some algorithms where you cut the task up to be a optimal number of pieces (e.g. sum an array in 3 bits knowing you have 3 cores to give it). You do need to do something to manage the number of threads. But don't overlook `std::async` it provides a basic way of doing the same thing that may suit. – Persixty May 15 '18 at 15:58