3

I am working on a program that handles mouse movement, it consists of two threads, the main thread gets the input and stores the mouse position in a fixed location and the child thread loops through that location to get the value:

void Engine::InputManager::MouseMove(const MouseMoveEvent& ev)
{
    cur_mouse_ev_.x_ = ev.x_;
    cur_mouse_ev_.y_ = ev.y_;
    cv_.notify_all();
}

void Engine::InputManager::ProcessInput(MouseMoveEvent* ev)
{
    while (true)
    {
        cv_.wait(u_mutex_);
        float dx = static_cast<float>(ev->x_ - pre_mouse_pos[0]) * 0.25f;
        float dy = static_cast<float>(ev->y_ - pre_mouse_pos[1]) * 0.25f;
        g_pGraphicsManager->CameraRotateYaw(dx);
        pre_mouse_pos[0] = ev->x_;
        pre_mouse_pos[1] = ev->y_;
    }
}

How do I reduce CPU utilization, I am using conditional variables to achieve this, is there a better way to do this? It seems that adding a delay to the subthreads would also work

2 Answers 2

4

Using std::condition_variable is a good and efficient way to achieve what you want.

However - you implementation has the following issue: std::condition_variable suffers from spurious wakeups. You can read about it here: Spurious wakeup - Wikipedia.

The correct way to use a condition variable requires:

  1. To add a variable (bool in your case) to hold the "condition" you are waiting for. The variable should be updated under a lock using the mutex.

  2. Again under a lock: calling wait in a loop until the variable satisfies the condition you are waiting for. If a spurious wakeup will occur, the loop will ensure getting into the waiting state again. BTW - wait method has an overload that gets a predicate for the condition, and loops for you.

You can see some code examples here: Condition variable examples.

A minimal sample that demonstrates the flow:

#include <thread>
#include <mutex>
#include <condition_variable>

std::mutex              mtx;
std::condition_variable cond_var;
bool                    ready{ false };

void handler()
{
    {
        std::unique_lock<std::mutex> lck(mtx);
        cond_var.wait(lck, []() { return ready; }); // will loop internally to handle spurious wakeups
    }
    // Handle data ...
}

void main()
{
    std::thread t(handler);

    // Prepare data ...
    std::this_thread::sleep_for(std::chrono::seconds(3));

    {
        std::unique_lock<std::mutex> lck(mtx);
        ready = true;
    }
    cond_var.notify_all();
    t.join();
}

Note:
Accessing the data (both when creating it and consuming it) must be done with proper synchronization (either holding the mutex or in any other appropriate way).
The minimum that is required is to hold the mutex while copying the data in the handler and then process the copy.
Atlertatively the data can be accessed without a copy, but with proper synchronization all the way.
This is left out of the example above for simplicity.

Sign up to request clarification or add additional context in comments.

4 Comments

I don't think that spurious wakeups are that big of an issue in this context, and they won't significantly reduce the CPU load. For the application of handling mouse movement, it would be much more interesting to collapse multiple mouse move events into one and only update the graphics to the last position. Further, in your code here, if you // Handle data ... without the mutex held, you get a race condition. You need to copy the shared data with the mutex held and then work with the copy instead. That part isn't totally clear here.
Regarding accessing the data - this is a good point and I just added a note about it (please feel free to edit and improve). Regarding spurious wakeups - this might be true in this specific case, but I wanted to demonstrate the general proper use of std::condition_variable.
Another point: what you wrote about handling mouse events is true in certain use cases but not all. For example if you want to use a mouse to draw a curve, you would like the system to process as many mouse event as possible to make the curve smooth.
That's true. In that case it would probably be best to queue every position and only prevent adjacent duplicates. A rendering thread could then pick up those changes. I was just making assumptions based on g_pGraphicsManager->CameraRotateYaw(dx) in OP's code.
0
  1. you could try using a semaphore, for (possibly) better performance
  2. instead of 2 threads, you could try using coroutines (standard or your own), for less memory consumption. A thread needs a stack frame, that's several MBytes at least. A coroutine may not need anything extra.

9 Comments

A thread doesn't need a stack frame. A thread needs an entire stack.
I prefer to write stack frame, as there is no actual data structure in use.
FWIW the thread-implementation itself uses data structures, some of which get pushed onto the beginning of the thread's stack.
Re, "I prefer to write stack frame." You will have a hard time talking to other software developers so long as you insist on calling things by whatever name suits your preference instead of using the names that everybody else uses.
The claim that a semaphore gives "(possibly) better performance" needs a justification. Even more since OP omitted a minimal reproducible example that actually shows how they use a mutex, you have nothing to compare to. That said, a thread doesn't necessarily need several megabytes. Lastly, the link to github which you provided is dead.
|

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.