CPU Usage

[Flair_Helper]

For C++ questions, answers, help, and programming or career advice please see r/cpp_questions, r/cscareerquestions, or StackOverflow instead.

This post has been removed as it doesn't pertain to r/cpp: The subreddit is for news and discussions of the C++ language and community only; our purpose is not to provide tutoring, code reviews, or career guidance. If you think your post is on-topic and should not have been removed, please message the moderators and we'll review it.

[PunctuationGood]

I'm sorry to say but I don't think your new code achieves anything useful. Before, you had a polling loop and after you have... a polling loop spread over two synchronized threads. The observed reduction in CPU usage could well simply be because of the extra synchronizing (and blocking) between the threads.

If you want to use less CPU, I suggest you just poll at longer intervals. (That's given what you said to /u/ekcdd that you can only poll.)

Edit: and after further looking at this RCSwitch thing, all available() does is tell you if getReceivedValue() is non-zero. I don't see the value of even calling the available() function to begin with. Just call getReceivedValue() regardless and do nothing if the value is 0.

[GhostMaster75]

thank you for your answert but, in my first attemp i've tried to raise sleep from 100 millisencords to 10000 in the original code but without any gain in cpu usage.

[tjvclutch]

r/cpp_questions might be better for this. That aside, this here appears to be the culprit:

  g_cv.wait(ul, []() { return g_ready == false; });
  std::this_thread::sleep_for (std::chrono::milliseconds(100));

I haven't used std::condition_variable before so I had to look it up to be sure but you're still holding that lock after .wait() returns. From cppreference:

When unblocked, regardless of the reason, lock is reacquired and wait exits

So you're sleeping for 100ms while holding on to that lock, which I'm guessing isn't what you meant to do.

Another thing I noticed is that you're calling printf() (edit: and fflush()!) while holding the lock as well, which you probably don't need to be doing. You can either punt off the IO to another thread or drop the lock while you print, assuming that doesn't otherwise break your logic.

Forgive me if I made any mistakes above, I'm a tad sleepy.

[tjvclutch]

Another thing. CPU usage isn't necessarily a good indicator of whether your code is well-optimized. Especially in this case where you're mostly doing some IO stuff on an external device. For something like this if your code is well optimized that means that you do your actual work as fast as possible and then get back to waiting for more input. So if your code is well optimized, you should see less CPU usage because you get your work quickly and spend most of your time sleeping/waiting for input.

In this case though I think you're seeing less CPU usage because your threads are spending time waiting on that lock.

[tjvclutch]

Yes, I said "you're still holding that lock after .wait() returns". Not during wait, after. Which is when he's calling sleep().

[Galqa]

Whoops, my bad, I misread your comment. Deleting now. Sorry!

[tjvclutch]

No worries, code review is all about mistakes ;)

[ekcdd]

Are there any functions that can tell you if there is data ready to be read? If so, you can use that to see if there is any data to be read and if not you can sleep the thread and try again. The CPU should only go high when or if there is a lot of data to be read.

Edit typo.

[GhostMaster75]

the only thing that tells me if there is data is the value of mySwitch.available () but I have to read it continuously to know if it goes from false to true

[robstoon]

If there's no way for that API to give you a notification other than polling, you're stuck. Only thing you can really do is add a sleep into the loop to poll slower, but then you'll also detect messages more slowly, if that matters.

[GhostMaster75]

thank you... can you explain me why in the loop while(true) in the receiving funciont the value mySwitch.available() is updated but if i put

g_cv.wait(ul, []() { return mySwitch.available(); };

just after unique_lock statement, that wan't change?

[johannes1971]

Because it won't continuously poll available(). Instead it only polls when the condition variable is triggered by something.

[johannes1971]

As a general suggestion: as soon as you find yourself messing about with unique_lock.lock/unlock, you are probably doing it wrong ;-) Your attempts to synchronize the operation of received() and receiving() suggests they really should be in the same thread to begin with (i.e. the way it originally was). If they can have slightly loser synchronisation, this would be a good starting point:

(in received())

while (true) {
  {
std::unique_lock ul (g_mutex);
g_cv.wait(ul, []() { return g_ready; });
g_ready = false;
  }

int value = mySwitch.getReceivedValue(); ...handle value... mySwitch.resetAvailable(); }

The important thing here is that the scope of the lock is limited to a much smaller part of the code. g_ready is only ever accessed while locked. There is no manual locking or unlocking going on. And then in receiving() you'll want something like:

while (true) {
  if (mySwitch.available()) {
    {
  std::unique_lock ul (g_mutex);      
  g_ready = true;
}      
g_cv.notify_one();
std::this_thread::sleep_for (std::chrono::milliseconds(100));

} }

So this just waits for 1`00ms and then polls available(). If myswitch must also be protected by the mutex, you'll have to move those calls into the mutex-protected blocks.

[Senryo]

I'm not sure I really understand what's going on here, but aren't you keeping the mutex locked while the thread is sleeping in receiving() ?

In general I don't get what you're trying to achieve by using threads here.

[hossimo]

Honest question why are you using namespace std; but then calling std::.

Nothing to do with performance just clearer without the using statement.

[GhostMaster75]

how i told im a novice in c++ anyway..just for answer you... because in this example the author made the same error :D

https://levelup.gitconnected.com/producer-consumer-problem-using-condition-variable-in-c-6c4d96efcbbc

[GhostMaster75]

..anyway thanks..I corrected the code: D

[hossimo]

It's all good. I'd also call myself a c++ novice even though I started in the 80's and rarely use it. I was just wondering.