patterncppMinor
Safe handling of variables in multi-threading application with shared resources
Viewed 0 times
handlingmultiapplicationsharedwiththreadingvariablessaferesources
Problem
I've been developing this application as my first C++ project and am at a point where it is functional and would like to take a step back and critique/improve my code from a standpoint of manageability and better coding practices. Additionally, it has some stability issues which I need to address, so I thought instead of dumping a ton of code out there and asking for feedback we'd start at the beginning.
This application has multiple threads with shared resources, and sharing these resources across threads was particularly tricky for a novice programmer. So please see the below 'main.cpp' of my application that creates and waits for the destruction of all threads. Additionally these notes might be helpful:
Thanks in advance, and all (constructive) input is appreciated.
```
int main()
{
std::cout exitsignal{ false };
std::atomic shutdownsignal{ false };
cv::Mat captureimage;
std::mutex capturemutex;
cv::Mat displayimage;
std::mutex displaymutex;
ProcessValues processvalues;
//Start threads
//Start image capture thread
std::thread t_imagecapture( CaptureImageThread,
&captureimage,
&capturemutex,
&exitsign
This application has multiple threads with shared resources, and sharing these resources across threads was particularly tricky for a novice programmer. So please see the below 'main.cpp' of my application that creates and waits for the destruction of all threads. Additionally these notes might be helpful:
- The threads are meant to be asynchronous and operate at different rates. For example, the thread that updates the display operates a lower frame-rate than the thread that actually captures the image. I particularly wanted to avoid the scenario where the threads were locked together and the slowest thread set the pace for all others.
- All threads are self exiting based on the 'exitsignal' variable. Essentially this code runs on raspberry pi in an automobile and when 12V signal is lost (car shut-off), a 3.3V input goes low and the GPIO thread writes 'exitsignal' true and all the other threads exit on their next iteration of their for-loop.
- The GPIO thread is finally exited by the 'shutdownsignal' written at the end.
Thanks in advance, and all (constructive) input is appreciated.
```
int main()
{
std::cout exitsignal{ false };
std::atomic shutdownsignal{ false };
cv::Mat captureimage;
std::mutex capturemutex;
cv::Mat displayimage;
std::mutex displaymutex;
ProcessValues processvalues;
//Start threads
//Start image capture thread
std::thread t_imagecapture( CaptureImageThread,
&captureimage,
&capturemutex,
&exitsign
Solution
Don't use threads to scatter your timing schedule of independent actions
In most cases timing schedules can be solved with a simple main loop and time measurement like such:
Threads and synchronization mechanisms come with some cost by means of performance and memory overhead you may want to avoid.
Minimize thread interdependencies
It seems a number of processing steps in your example could be serialized trivially by calling these one after another (has finishied).
You're passing
Such could be easier solved with a simple function calling sequence.
Reduce the number of threads to the parts that could be run independently
It seems your scenario can be reduced to 3 threads. One that takes the video capture input, one that does the image processing (depends if another CPU core is available), and the last one that feeds the video output.
You likely can synchronize all these using a (
In that mentioned scenario a
In most cases timing schedules can be solved with a simple main loop and time measurement like such:
bool exit = false; // In case you have concurrency here a `std::atomic`
// is fine of course
auto least = std::chrono::high_resolution_clock::now();
do {
auto current = std::chrono::high_resolution_clock::now();
auto timeDiff = current - least;
if(timeDiff >= 20ms) {
// do the action every 20 milliseconds
}
if(timeDiff >= 1s) {
// do the action every second
}
// aso.
} while(!exit);Threads and synchronization mechanisms come with some cost by means of performance and memory overhead you may want to avoid.
Minimize thread interdependencies
It seems a number of processing steps in your example could be serialized trivially by calling these one after another (has finishied).
You're passing
cv::Mat captureimage; as parameter all of the time, but the processing steps are separate threads that have to wait for each others processing step.Such could be easier solved with a simple function calling sequence.
Reduce the number of threads to the parts that could be run independently
It seems your scenario can be reduced to 3 threads. One that takes the video capture input, one that does the image processing (depends if another CPU core is available), and the last one that feeds the video output.
You likely can synchronize all these using a (
std::mutex protected) processing queue of std::queue frames.In that mentioned scenario a
std::condition_variable may come in handy to trigger the concurrently waiting std::threads execution, as soon a new captured frame is available, or it's readily processed for output.Code Snippets
bool exit = false; // In case you have concurrency here a `std::atomic<bool>`
// is fine of course
auto least = std::chrono::high_resolution_clock::now();
do {
auto current = std::chrono::high_resolution_clock::now();
auto timeDiff = current - least;
if(timeDiff >= 20ms) {
// do the action every 20 milliseconds
}
if(timeDiff >= 1s) {
// do the action every second
}
// aso.
} while(!exit);Context
StackExchange Code Review Q#151253, answer score: 2
Revisions (0)
No revisions yet.