thread problem

>>>> if I don't want to use a thread, how can I call a second code routine
>>>> from the first routine without having to wait for the second code
>>>> routine to return in order to terminate or reloop the first routine?

You only could do that by running some kind of infinite message loop in the first (and only) thread which would do both the job of first thread and the second job avoiding any lengthy operations but doing partial jobs only:

// main thread

     while (job = peekForJob())
     {
             job->makeJob();
     }

The peekForJob would check a job queue for new 'jobs'. If the job queue was empty it would wait a little time to not using all CPU while looping. The peekForJob would end if a 'quit' job was detected. All 'jobs' were class object pointers derived from some common baseclass 'Job'. The makeJob was a virtual function.  

Note, to make the above work all implementations of makeJob must have short execution time only and may have no waits or infinite loops. In case of a 'length' job it must parted into small pieces, e. g. making only one iteration of a loop for each call.
A job that couldn't be solved with one call would do a little part and put a new job at end of the job queue to continue work.  

I am not sure if for image processing a 'job pump' is the right design. It seems to me that using two separate threads is much easier. To solve the CPU issue you simply could add a little sleep as Ike suggested. That applies for any infinite loop whether it is in the main thread or in the worker thread.

Regards, Alex

>> 1. how to evaluate the problem of thread CPU usage

You probably have a busy wait somewhere, no ? Using sleep instead (as has already been suggested) should fix that.

I have determined that it is the while loop in the thread that is causing excessive CPU usage. My strategy now is to turn the thread on and off as needed. The sleep command, while done without error, did not improve the usage. The thread is run successfully with these commands:

MVMFrm.cpp
===========
m_Tracker_Mod= new Tracker_Mod( );              //instantiate thread
if ( m_Tracker_Mod->Create() != wxTHREAD_NO_ERROR )         {
         MessageBox(NULL,"Thread Error!","Error",MB_OK);
               wxExit( );
         }
      //start Tracker_Mod thread
         if ( m_Tracker_Mod->Run() != wxTHREAD_NO_ERROR )
        {
            wxExit( );
         }

Now,my question concerns runing  the thread from a different .cpp file (same exe) :

MVMFrm.h
============
private:
        Tracker_Mod*      m_Tracker_Mod;

MVMFrm.cpp
===========
m_Tracker_Mod= new Tracker_Mod( );              //instantiate thread
if ( m_Tracker_Mod->Create() != wxTHREAD_NO_ERROR )
         {
         MessageBox(NULL,"Thread Error!","Error",MB_OK);
               wxExit( );
         }

Otherfile.cpp
===========
void FrameCallBack( TProcessedDataProperty* Attributes, unsigned char* Frameptr )
{
Tracker_Mod* p_Mod;
p_Mod->Run(); //<------------------ERROR
}

ERROR:
 [Error C2027] use of undefined type 'Tracker_Mod' see declaration of Tracker_Mod
 [Error C2227] left of '->Run' must point to class/struct/union/generic type

QUESTION: How do I correctly point to the m_Tracker_Mod instance to run it in FrameCallBack?

Thank you,

WLE

>> I have determined that it is the while loop in the thread that is causing excessive CPU usage.

Can you show that while loop ?


>>  [Error C2027] use of undefined type 'Tracker_Mod' see declaration of Tracker_Mod

Where and how is Tracker_Mod defined ? Did you include the header that contains its definition in the Otherfile.cpp file ?

Tracker_Mod::~Tracker_Mod( )
{
      m_pMutex=NULL;


}

////////////////////////////////////////////////////////////////////
// Method:      On Exit
// Class:      Tracker_Mod
// Purpose:      do soemthing on exit
// Input:      nothing
// Output:      nothing
////////////////////////////////////////////////////////////////////
void Tracker_Mod::OnExit( )
{
      // destroy - clean my place

      delete( m_pMutex );

}


void *Tracker_Mod::Entry( )
{
      int i = 0;
      m_bLife = 1;

      ////////////////////////////////////////////////////////////////
      // Start Life Cycle
      ////////////////////////////////////////////////////////////////
      // loop as long as flag m_bLife = 1
      while( m_bLife )
      {
            do stuff.....//<-----------------------my code goes here

      }

 return NULL;
}


**If the while loop is removed and make it "one shot" per thread the CPU usage drops dramatically. It doesn't matter whether I put anything in the while loop above as it is...even with a skeleton loop it stil tops out 99% CPU usage.

**Tracker_Mod is defined in MVMFrm.h and implemented (instantiated) in .cpp (see above)

left out this in MVMFrm .h:

class Tracker_Mod;

>> My strategy now is to turn the thread on and off as needed

I think you'll find using an Event kernel object is the simplest way to do this. The thread will block on the event until the main thread 'triggers' it. At this point your worker threads does some work and then goes back to blocking on the event until it is needed again...

Win32 API...

http://msdn2.microsoft.com/en-us/library/ms682396.aspx

MFC...

http://msdn2.microsoft.com/en-us/library/ms682396.aspx

-Rx.

Example on how to use the Event object...
http://www.codeproject.com/win32/Win32_Event_Handling.asp

>> even with a skeleton loop it stil tops out 99% CPU usage.

As I thought : you have a busy wait ... In other words, there is always something to do (even if it's just looping and doing nothing), so the thread will take as much CPU as possible.

You should add a sleep somewhere in the while loop where appropriate. What is supposed to happen in that loop ?


>> **Tracker_Mod is defined in MVMFrm.h

Did you include that header in the Otherfile.cpp file ?


>> class Tracker_Mod;

I hope you have a bit more than that ... this is just a forward declaration of the class, it is not the actual declaration ... That should look something like this :

        class Tracker_Mod {
            // your members and methods
        };

>> You should add a sleep somewhere in the while loop where appropriate
Or just use an Event object to block the thread until it's need it?

>> >> You should add a sleep somewhere in the while loop where appropriate
>> Or just use an Event object to block the thread until it's need it?

Anything that puts the thread on hold would be ok (I used sleep as a general term).

But sleep only yields the thread it doesn't block it; so it still gets scheduled time! Depending upon how the loop is written there is the risk of causing thread starvation with lower priority processes. Blocking on a kernel synchronization object (in this case an event) means the thread will never be scheduled any time until the main thread releases it. Once released it will immediately do it's work.

It really depends on what the thread does (that's why I asked about that earlier, Wanderinglazyeye). Since an infinite loop was used, I assumed that there was something that actually needed to be done all the time. In most cases, a normal sleep would be sufficient, as it leaves it up to the scheduler to decide which process/thread needs the most CPU.

Waiting for more info ...

:-p

>>>> Or just use an Event object to block the thread until it's need it?
If using an event (handle) you need to create one with CreateEvent, e. g. in the main thread,  and in the worker thread use one of the wait functions like WaitForSingleObject to wait for getting 'kissed'. The problem is, if you no one was signaling the event you wait for ever if setting the WaitForSingleObject to INFINITE wait. If using a timeout the whole thing is not so much different to a 'little' sleep but needs much more code.

A. use a sleep

       // loop as long as flag m_bLife = 1
      while( m_bLife )
      {
            do stuff.....//<-----------------------my code goes here
            Sleep(10);   // wait 10 milliseconds before going on
      }

B. wait for event

       // loop as long as flag m_bLife = 1
      while( m_bLife )
      {
            do stuff.....//<-----------------------my code goes here
            // e. g. have a timeout of 10 seconds
            DWORD ret = WaitForSingleObject(eventHandle, 10000);
            if (ret == WAIT_TIMEOUT)
            {
                  // did they forget me?
                  // what to do?
                  // is it an error or normal if no one is signaling the event
            }
      }

You see, an event driven handling  requires some other thread to have some kind of control over the current worker thread. It is much less simple than a polling loop with a little wait (A). Take the 'job queue' I described above. If for every new job added to the job queue the thread event was signaled by that thread which had added the new job, you could do the B thing using an INFINITE wait. If the main threads wants the worker thread to stop it would put the 'stop job' to the queue and signal the event. Then recognizing the stop job finally would break the infinite loop. However, as told, a simple sleep might do the same but is much easier.

Regards, Alex

I don't agree that it's easier!

As already pointed out, it runs the risk of causing thread starvation and is CPU wasteful as the thread is always scheduled. If this is a short term loop (something that isn't going to run for long) then I might agree but if it is something designed to run for long periods then, IMHO, just using a sleep isn't the way to do this. Why do you need the timeout? As long as the main thread is running and signalling when there is work to be done (which is what it should be doing) then so is the process so the worker will just sleep until signaled. If the main thread terminates so will the process (and therefore the worker). Of course, the main thread needs to signal and wait for the worker to finish before it does -- just like if it was a sleep loop.

Multi-threaded programming (done properly) isn't easy -- we shouldn't presume it is!

Of course, using an Event does have a cost (transition overhead from user space to kernel space and vice versa) but since this isn't a performance critical process (evident by the fact that the latency involved in the sleep/poll solution is acceptable) the cost isn't really a concern!

Putting a sleep statement in has no appreciable effect on the CPU usage - still at 99%.

I included the tracker_mod.h and it compiles. Now I get a run time error:

Access violation at blah blah

If I comment out this line it goes away.

p_Mod->Run();

I want to refocus the discussion on fixing this and running a thread from Othermodule.cpp:

Repost of relevant code (in my estimation : ) )
OtherModule.h
================
...

OtherModule.cpp
================
#include "MVMFfm.h"
extern Tracker_Mod*      m_Tracker_Mod;
void FrameCallBack( TProcessedDataProperty* Attributes, unsigned char* Frameptr )
{
Tracker_Mod* p_Mod;
p_Mod->Run();
}

MVMFrm.h
===========
class Tracker_Mod;
...
private:
        Tracker_Mod*      m_Tracker_Mod;

MVMFrm.cpp
===========
m_Tracker_Mod= new Tracker_Mod( );              //instantiate thread
if ( m_Tracker_Mod->Create() != wxTHREAD_NO_ERROR )
         {
         MessageBox(NULL,"Thread Error!","Error",MB_OK);
               wxExit( );
         }

Thread.cpp
========

already works and is posted above

WLE

Putting a sleep statement in has no appreciable effect on the CPU usage - still at 99%.

I included the tracker_mod.h and it compiles. Now I get a run time error:

Access violation at blah blah

If I comment out this line it goes away.

p_Mod->Run();

I want to refocus the discussion on fixing this and running a thread from Othermodule.cpp:

Repost of relevant code (in my estimation : ) )
OtherModule.h
================
...

OtherModule.cpp
================
#include "MVMFfm.h"
extern Tracker_Mod*      m_Tracker_Mod;
void FrameCallBack( TProcessedDataProperty* Attributes, unsigned char* Frameptr )
{
Tracker_Mod* p_Mod;
p_Mod->Run();
}

MVMFrm.h
===========
class Tracker_Mod;
...
private:
        Tracker_Mod*      m_Tracker_Mod;

MVMFrm.cpp
===========
m_Tracker_Mod= new Tracker_Mod( );              //instantiate thread
if ( m_Tracker_Mod->Create() != wxTHREAD_NO_ERROR )
         {
         MessageBox(NULL,"Thread Error!","Error",MB_OK);
               wxExit( );
         }

Thread.cpp
========

already works and is posted above

WLE

>> Putting a sleep statement in has no appreciable effect on the CPU usage - still at 99%.
*cough* no, really -- pretends to be surprised! Like I said the thread is still scheduled time -- so it still uses CPU!

>> >> Putting a sleep statement in has no appreciable effect on the CPU usage - still at 99%.
>> *cough* no, really -- pretends to be surprised! Like I said the thread is still scheduled time -- so it still uses CPU!

If you put a sleep in the while loop of the thread that was previously consuming 99% of the CPU, and if the rest of the loop code doesn't take a long time, then your thread WON'T take up 99% of the CPU any more. During the time it's "sleeping" another thread gets the chance to use some CPU. That's the whole point of making a thread sleep.

Are you sure that your application is correctly designed ? Can you describe the threads that are running, and what they're doing ? Can you also show the compleet while loop ?

>> During the time it's "sleeping" another thread gets the chance to use some CPU
But... you either have to sleep for long enough to make sure you get the benefit (adding latency to your code) or too short a time in which case the thread will keep getting scheduled and, if there is no other higher priority thread, it will get given a CPU quantum -- this includes all the thread context transition that must happen for the thread to be woken up only for it to sleep again. This is consistent with what is being seen! All this takes CPU time and is wasteful and since the code to implement an Event isn't much harder I just cannot see any real reason for coding it this way. This would only be a suitable paradigm if the loop was to last for a very short time.

>> This would only be a suitable paradigm if the loop was to last for a very short time.

Exactly, but I'm still waiting for Wanderinglazyeye to give that information.

Understood.

>>>> Tracker_Mod* p_Mod;
>>>> p_Mod->Run();

Of course it will crash. You didn't initialize the local p_Mod pointer, e. .g with

    p_Mod = new Tracker_Mod();


Note, normally a callback function uses the pointer argument (what was framePtr in your case). Using a locally defined pointer which points nowhere looks like 'it wasn't designed' but 'playing around'.

I agree do that what Infinity said. If a Sleep(10) within the while loop doesn't decrease CPU consuming to some reasonable 5 percent or so, there is some other lengthy doing in  

      do stuff.....//<-----------------------my code goes here

Note, if running an infinite loop in a thread you need to give other threads a chance in every lengthy loop. Either by sleeping a little while or by waiting for an event.

>>>> *cough* no, really -- pretends to be surprised!
>>>> Like I said the thread is still scheduled time -- so it still uses CPU!

evilrix, that is nonsense. A Sleep within the loop makes the thread being put off executing. And all other threads will get scheduled until finally the waiting thread may get a second chance. CPU consuming of the thread will go to 0 while waiting. It only depends on the 'do stuff' whether it is consuming still a relevant piece of CPU after adding a Sleep statement.

Regards, Alex

>>>> But... you either have to sleep for long enough
>>>> to make sure you get the benefit

Sleep is dealing for milliseconds. Thread scheduling is made in nanoseconds. It is always long enough to give other processes a chance.

>>>> since the code to implement an Event isn't much harder

???
An event needs a second thread which controls the worker thread. That is a new game.

>> that is nonsense

Um, no it is not!

Yes it will go to 0 whilst waiting, but read my point above again! You either have to wait long enough for this to be worth while (introducing unnecessary latency) or not enough so you'll chomp CPU for no good reason. If there are no other higher priority threads then this thread will get rescheduled -- in preference to lower priority threads. This thread wakes up (this costs an unnecessary thread transition) and has to perform whatever work it needs to to decide whether to sleep again or not before it is put back to sleep (another unnecessary thread transition). I'm not saying don't do this, I'm saying don't do it if it's likely to run for a long while as it will keep stealing quantum's from lower priority threads! that may result in performance degradation on other processes. Blocking on an event takes no CPU -- it's never scheduled.

>> Sleep is dealing for milliseconds. Thread scheduling is made in nanoseconds. It is always long enough to give other processes a chance.
The very fact that it must wake up and do unnecessary work means it is stealing cycles from other threads.

>> An event needs a second thread which controls the worker thread
Yes, the main thread!

>>>> Yes, the main thread!
Which is running an infinite loop with a sleep?

>> Which is running an infinite loop with a sleep?

Sorry, I was under the impression we were talking about a main thread and a worker thread -- this is what the question originally implied and this what I have been basing my reasoning on.

My suggestion of an event was based upon this...

>> My strategy now is to turn the thread on and off as needed

"I think you'll find using an Event kernel object is the simplest way to do this."

This was the paradigm I thought we were discussing

Main thread collates some work for worker thread to do and queues it.
Main thread signals worker
Worker does work, whilst main thread goes back to collating next job for worker
Worker finishes and goes back to blocking
Repeat

>>>> My suggestion of an event was based upon this...

Ok. Granted.

I like events better as well. But not using events isn't the reason for 100% CPU consuming. And there is nothing bad with running an infinite loop with a sleep. As told, the main loop of the main thread rarely can do somewhat else but polling. And waiting for a timer event may sound better than 'sleeping' but actually there is no difference beside that the one needs ten times of code than the other.

>> not using events isn't the reason for 100% CPU consuming
No, I agree there is likely to be other issues

>> beside that the one needs ten times of code than the other
To add support for an event? 10 lines more code maybe, not 10 times! :-D

The problem with the sleep as I see it is this...

If the cost for the worker to check it has something to process is expensive (do we even know what that is?) then you have a decision to make. Do I sleep for short time so as not to miss work but run the risk of lots of costly checks or do I sleep for longer but run the risk of missing work (adding latency )? This is what I meant about sleeping long enough (I guess I didn't word it very well). You have to balance out the cost of waking vs the cost of not. Every time it wakes it steals unnecessary CPU and this is probably not ideal. With an event it'll just get waken up and do it.

Anyway, I agree both solutions have there merits -- pros and cons. I think we both understand where the other is coming from now -- it seems mis-communication may have hindered this :)

-Rx.

s/there/their -- grammar -- tsk!

>>>> I think you'll find using an Event kernel
>>>> object is the simplest way to do this.

No. A sleep is much simpler. The only question is if a sleep is as efficient than waiting for an event. Apparently, if an event was signaled the waiting thread was scheduled much faster than if waiting for the time period to expire and checking a shared 'flag' for the event. But, assuming it has to wait for 100 cycles on average before being awaked, the (little) time delay actually doesn't play a role. It even may be contraproductive if the scheduled thread maybe was blocked immediately after being scheduled by a mutex. If waiting for the time to elapse, the main thread would have enough time to release the mutex. By using WaitForMultipleObjects (or by using WaitForSingleObject with a timeout) you could combine the sleep with event triggers *but* even if it may be more efficient, it is much more complex.

>>>> 10 lines more code
10 lines to 1 line is ten times.

>> 10 lines to 1 line is ten times.
Muhahahaha -- I think that over-simplifies it a little :)

Anyway, I'm gonna leave it there... I really must get to sleep (my wife is gonna kill me!).

Night Alex, thanks for the chat.

>>Exactly, but I'm still waiting for Wanderinglazyeye to give that information.

Sorry, wasn't trying to be coy with the code. The "do stuff" code is below.

>>I agree do that what Infinity said. If a Sleep(10) within the while loop doesn't decrease CPU consuming to some reasonable 5 percent or so, there is some other lengthy doing in  

      do stuff.....//<-----------------------my code goes here

No lengthy doing...consumes 99% CPU when there is *no code* for do stuff, just while loop (skeleton).

I would like to try turning the thread on and off as needed, but now I get this error after Alex' suggestion:

Cannot resume thread 0 (error 6: the handle is invalid).

"Do stuff" code:

IplImage* ATImage = 0;
                                                CvMemStorage* storage = cvCreateMemStorage(0);
                                                //CvMemStorage* childstorage = cvCreateChildMemStorage(storage );
                                                CvSeq* contours = 0;
                                                ATImage = cvCreateImage(cvSize(i_Width,i_Height), 8, 1); //Create Image
                                                contour_image = cvCreateImage(cvSize(i_Width,i_Height), 8, 1); //Create Image
                                               cvThreshold( NIRImage, ATImage, 30, 125, CV_THRESH_BINARY );
                                                //cvFindContours( ATImage, storage, &contours, sizeof(CvContour),CV_RETR_EXTERNAL, CV_CHAIN_APPROX_NONE, cvPoint(0,0) );
                                               CvContourScanner blobs = cvStartFindContours(ATImage, storage,sizeof(CvContour), CV_RETR_EXTERNAL, CV_CHAIN_APPROX_SIMPLE, cvPoint(0,0) );
                                               contours = cvFindNextContour(blobs);
                                               contours = cvEndFindContours( &blobs );
                                               cvDrawContours( contour_image, contours, CV_RGB(0,0,255),CV_RGB(0,255,0), 0, 1, CV_AA, cvPoint(0,0) );
                                               cvReleaseImage(&ATImage);
                                               cvReleaseMemStorage( &storage );

                                               if (!StartMachineVisionInst.preview_on( false, false))
                                               cvReleaseImage(&contour_image);

Here is my solution so far. So far, it works well enough that CPU usage is between 11-16% in full effect. I basically invoke the Machine Vision  thread after every acquired  image from the camera; sort of fire and forget. As long as the MV routine is faster than the image acquirement from the camera, we're good. I am sure delay code could be written if the reverse were true.

Drawback is that everytime I call the StartMachineVisionInst.Start_Tracker() routine, I create the thread everytime. I would prefer to create the thread once and only run it over and over again. If I put the m_Tracker_Mod->Create() outside of the function I get a windows handle error when I call it. Any insight on further streamlining this approach would be appreciated.

void FrameCallBack( TProcessedDataProperty* Attributes, unsigned char* Frameptr )
                                    {
                                            //int i_Width, i_Height,
                                            int cam_bin_mode, key;
                                            int Time_Stamp_Now=0, Last_Time_Stamp=0, Frame_Interval=0, Frame_Rate=0;
                                             cam_bin_mode=Attributes->Bin;
                                            ....blah blah aquire image stuff
                                            StartMachineVisionInst.Start_Tracker();
                                    } //end of call
bool StartMachineVision::Start_Tracker()
{
    m_Tracker_Mod= new Tracker_Mod();
    m_Tracker_Mod->Create();
    m_Tracker_Mod->Run();

   return true;
}

WLE...

>> I really must get to **sleep**

lol ... Now that's funny ... (don't know if it was intended though ;) )


>> No lengthy doing...consumes 99% CPU when there is *no code* for do stuff, just while loop (skeleton).

Then you didn't put in the sleep, or some other thread is consuming 99% of the CPU.



>> "Do stuff" code:

How long does that code take on average (when run once) ?


>> I would prefer to create the thread once and only run it over and over again.

Then keep the thread running, and wake it up whenever you have something to do for it ... You can wake it up with an event, or by simply setting some global flag (protected by a mutex if needed).

>> or by simply setting some global flag (protected by a mutex if needed).

a) If this approach is taken it MUST be declared volatile otherwise the compiler will be at liberty to optimize it. The value may be cached on a per thread basis and you'll not see the changes inter-thread!

b) AFAIK you don't need to protect type sig_atomic_t with a mutex

sig_atomic_t <csignal> Integral type: This is an integral type of an object that can be accessed as an atomic entity, even in the presence of asynchronous signals.

b) If it is necessary to implement a mutex to protect global flag surely it's just a easy to use an event? :-p

-Rx.

>> b) If it is necessary to implement a mutex to protect global flag surely it's just a easy to use an event? :-p

They're really two different approaches - in case of the flag, the thread needs to poll whether the flag is changed. In case of an event, you need to add an event mechanism. I wouldn't say one is easier than the other - they're just different.

>>>> it MUST be declared volatile otherwise the
>>>> compiler will be at liberty to optimize it.
The flag should be a member of the object initially passed by pointer to the thread. No need for volatile then. It also could be a static member of any class and volatile isn't needed either.

class Globals
{
public:
      static bool stopFlag;
      ...
};

// globals. cpp

#include "globals.h"
      ...
     bool Globals::stopFlag = false;  // init

// anythread.cpp

   while (!Globals::stopFlag)
   {
         doStuff();
         Sleep(10);
   }

b) AFAIK you don't need to protect type sig_atomic_t with a mutex

sig_atomic_t is not a portable type. In case of setting/checking bool flags you don't need a mutex nor atomic setting. It doesn't matter whether the bool was recognized at the current iteration or the next.

>>>> c) If it is necessary to implement a mutex to
>>>> protect global flag surely it's just a easy to use an event? :-p
It isn't necessary.

>> They're really two different approaches - in case of the flag, the thread needs to poll whether the flag is changed. In case of an event, you need to add an event mechanism. I wouldn't say one is easier than the other - they're just different.back to top

Adding the event mechanism isn't hard (to be honest I'm surprised it is, ostensibly,  being argued against) and, in my opinion, will be less problematic and, I suspect more efficient  for the reasons I've already mooted above. I am just pointing out if you are going to start using other kernel objects to solve this problem then why not just use the one that gives the simplest and probably most efficient solution -- in my opinion that is using an event. This kind of problem is exactly what the event object is for isn't it?

Anyway, I think we've done this polling vs. event to death now -- the masses have worn me down. Although I still don't agree with them, I can't be bothered to continue arguing against anymore! :-p

>> I am just pointing out if you are going to start using other kernel objects

What other kernel objects are you talking about ? I didn't mention any ... If you're referring to a mutex, then that's just a general term, and can be implemented any way you please.


>> Anyway, I think we've done this polling vs. event to death now -- the masses have worn me down.

To me it's not a matter of which is better always, but a matter of which is better in this situation. And I can't answer that until I have more information.

>> sig_atomic_t is not a portable type.

Um, it is defined in the C89 and C++2003 standard docs!

"The C + + Standard library provides 19 standard types from the C library, as shown in Table 97:
...
sig_atomic_t
..."

>> The flag should be a member of the object initially passed by pointer to the thread. No need for volatile then...

Straight from the C+ 2003 standard...

"When the processing of the abstract machine is interrupted by receipt of a signal, the values of objects with
type other than volatile sig_atomic_t are unspecified, and the value of any object not of
volatile sig_atomic_tthat is modified by the handler becomes undefined."

Did I misunderstand this text?

>> What other kernel objects are you talking about ? I didn't mention any ... If you're referring to a mutex, then that's just a general term, and can be implemented any way you please.

Without reason to believe otherwise, I think it was reasonable for me to assume you were referring to a kernel mutex. Of course, a critical section could also be used.

Sorry, to clarify, my reference for my post where I quited the standards doc was...

INTERNATIONAL ISO/IEC
STANDARD 14882
Second edition
2003-10-15

>> it is defined in the C89 and C++2003 standard docs

It is also defined in the 1st edition, ISO/IEC 14882 - 01/09/1998

>>>> Um, it is defined in the C89 and C++2003 standard docs!
I have three compilers here: VC6, VC7, xlc.
Type sig_atomic_t is defined for all these as an 'int' type (in signal.h). At AIX xlc it is defined volatile additionally. So, the existence of the type 'sig_atomic_t' doesn't seems to imply that the types really can be set in an atomic operation cause I know for sure that incrementing a shared int was made in more than one assembler statements at VC6 and VC7, so it is not an atomic operation. I would assume that sig_atomic_t only means that *all bits* of the type were written with one operation, i. e. you are reading either the old contents or the new contents but not a partially written contents.  So you are right that it is a portable type, but it gives no more functionality than a normal integer at least at the compilers I know.  

>>>> Did I misunderstand this text?
Most likely. The text seem to refer to signal processing what is off topic here. A bool properly initialized and set only has 0/1 value regardless if volatile or not. So, the only question is whether the compiler would optimize a

     while (global_or_static_flag)
     {
           ...
     }

to not checking the while condition for each iteration if the 'global_or_static_flag' was not volatile. The answer is 'no'. IMO, there is no (longer a) C/C++ compiler on the whole earth which would optimize that in a multithreaded environment. I experienced compilers in the past which had problems regarding that issue. But I didn't use volatile keyword at least for the last 12 years, maybe longer, for hundreds of projects on different platforms most of them multi-threaded and never experienced any problem doing so. Hence, you can define it volatile or not, it makes no difference.

I can see this is going nowhere fast! You have clearly formulated an opinion on this that is immutable - but one to which I do not subscribe. That is fine, we are both entitled to opinion; however, my opinion is based on well documented fact and my own personal (bad) experience. To this end I will no longer take part in this thread (I see little point as it's not actually working towards answering the original question); however, I will close my end of this discussion with some authoritative sources that I have drawn on to support my assertions so that you may draw your own conclusions from them.

May thanks for the opportunity to discuss this with you Alex, I look forward to our next encounter :)

"February 01, 2001
volatile - Multithreaded Programmer's Best Friend
Andrei Alexandrescu

The volatile keyword was devised to prevent compiler optimizations that might render code incorrect in the presence of certain asynchronous events. For example, if you declare a primitive variable as volatile, the compiler is not permitted to cache it in a register -- a common optimization that would be disastrous if that variable were shared among multiple threads. So the general rule is, if you have variables of primitive type that must be shared among multiple threads, declare those variables volatile."

http://www.ddj.com/cpp/184403766

"Consider the following class thats used in a multithreaded application:

class A
{
private:
 bool b;
public:
 void wait()
 {
 while (!b)
 {
  Sleep(1000); // sleep for 1 second
 }
 }
 void wakeup()
 {
 b = true;
 }
};

A::wait() checks the value of b every second, and returns only if that variable has been set to true by another thread. Most compilers will notice that Sleep() cant modify the variable b, and that wait() doesnt modify b either. Its therefore tempting to optimize the code by caching b in a register instead of accessing it from the much slower on-board memory. Alas, this optimization causes a new problem. When a different thread modifies b, the change will not be reflected in the register that caches b. Consequently, the while-loop will run forever. To avoid this bug, the programmer must explicitly declare b as volatile:

class A
{
private:
 volatile bool b;
public:
//..same as above
};This will prevent the caching of b in a register, forcing the program to read the correct value of b slowly but surely from the on-board memory. If you understand this, youve practically understood the role of the volatile keyword."

http://www.informit.com/guides/content.aspx?g=cplusplus&seqNum=345

"The volatile keyword is a type qualifier used to declare that an object can be modified in the program by something such as the operating system, the hardware, or a concurrently executing thread."

http://msdn2.microsoft.com/en-us/library/12a04hfd(VS.80).aspx

"sig_atomic_t integer Type of object that can be modified as atomic entity, even in presence of asynchronous interrupts; "

http://msdn2.microsoft.com/en-us/library/323b6b3k(vs.71).aspx

"To avoid uncertainty about interrupting access to a variable, you can use a particular data type for which access is always atomic: sig_atomic_t. Reading and writing this data type is guaranteed to happen in a single instruction, so there's no way for a handler to run "in the middle" of an access.

The type sig_atomic_t is always an integer data type, but which one it is, and how many bits it contains, may vary from machine to machine."

http://www.delorie.com/gnu/docs/glibc/libc_496.html

Kind regards,

-Rx.

Fascinating discussion. Well, the practical implications were: I liked the global flag idea better than my own code, so I implemented it.

Result: I can turn the thread off, but it doesn't wake when I change the flag back.

#include "global.h"
Tracker_Mod::Tracker_Mod() : wxThread( wxTHREAD_DETACHED )
{
      // get frame refrence

      // start life
      m_bLife = 1;

      m_pMutex = new wxMutex( );



      return;
}

////////////////////////////////////////////////////////////////////
// Method:      Destructor
// Class:      Tracker_Mod
// Purpose:      object destructor
// Input:      nothing
// Output:      nothing
////////////////////////////////////////////////////////////////////
Tracker_Mod::~Tracker_Mod( )
{
      m_pMutex=NULL;


}

////////////////////////////////////////////////////////////////////
// Method:      On Exit
// Class:      Tracker_Mod
// Purpose:      do soemthing on exit
// Input:      nothing
// Output:      nothing
////////////////////////////////////////////////////////////////////
void Tracker_Mod::OnExit( )
{
      // destroy - clean my place

      delete( m_pMutex );

}



////////////////////////////////////////////////////////////////////
// Method:            Entry
// Class:            Tracker_Mod
// Purpose:            the main executable body of my thread
// Input:            nothing
// Output:            void pointer
////////////////////////////////////////////////////////////////////
void *Tracker_Mod::Entry( )
{
      int i = 0;
      m_bLife = 1;

      ////////////////////////////////////////////////////////////////
      // Start Life Cycle
      ////////////////////////////////////////////////////////////////
      while (!Globals::stopFlag)
      {
        MessageBox(NULL,"Thread On!","Error",MB_OK);

        wxThread::Sleep(10);
    }
      

    return NULL;
}

===
global.h/.cpp as written by Alex
GUI button false/true to toggle thread, logic working fine.

WLE

>>>> Result: I can turn the thread off,
>>>> but it doesn't wake when I change the flag back.

You would need two flags to switch it on off:

{
      int i = 0;
      m_bLife = 1;

      ////////////////////////////////////////////////////////////////
      // Start Life Cycle
      ////////////////////////////////////////////////////////////////
      while (!Globals::stopFlag)
      {
            static bool firston = true;
            static bool firstoff = true;
            if (Globals::goonFlag)
            {
                 if (firston)
                 {
                     MessageBox(NULL,"Thread On!","Error",MB_OK);
                     firston = false;
                     firstoff = true;
                  }
                 doStuff();
            }
            else
            {
                 if (firstoff)
                 {
                     MessageBox(NULL,"Thread Off!","Error",MB_OK);
                     firstoff = false;
                     firston = true;
                  }
             }
        wxThread::Sleep(10);
    }
    return NULL;
}
     
The firston, firstoff is to have the messagebox only when toggling.

Regards, Alex

>> You would need two flags to switch it on off:

Not really (unless I misunderstood the requirements) :

        while (1) {
            while (!work_available) {
                // sleep for a while
            }
            // do the useful stuff
            work_available = false;
        }

with work_available the global that signifies that there is work to do for the thread.

>> To this end I will no longer take part in this thread
This was not an attempt at a pun -- and I specifically meant the argument between Alex and I.

>>  can turn the thread off, but it doesn't wake when I change the flag back
The thread may not be seeing the flag being changed by the main thread if the type hasn't been declared volatile. You might even get inconstant behavior between debug and release builds as debug builds are not generally optimized; whereas, release builds generally are.

Consider using...

#include <csignal>
class Globals
{
public:
      static sig_atomic_t volatile stopFlag;
};


volatile means it will not be cached in the register so all threads will see the change.

sig_atomin_t is a portable type (x-platform) that guarentees the type can be updated atomically (one CPU instruction). Although, in principle this should be true of any integer type using sig_atomin_t ensures you'll be using the correct type for the platform the code is built on.

NB. The standard talks about signals not thread when discussing the use of this type; however, this is because ANSI C++ has no concept of threads (as far as the standard is concerned) but it does discuss interrupts and a change of thread context is still an interrupt.

I hope this helps (and doesn't start another argument).

Regards,

-Rx.

Whilst you are looking at boost you might also want to look at boost::threads...

http://www.boost.org/doc/html/thread.html

NB. I do not work for boost.org I am just a big fan :)

>>>> Not really (unless I misunderstood the requirements) :
>>>> while (1) {

I like

   while (!Globals::stopFlag)

better. A 'while(1)' is so ... inifinite   ;-)

But of course for the switching we do need only one flag.
 

>> better. A 'while(1)' is so ... inifinite   ;-)

Ah, that's the second flag ... heh ... time to go to bed again I guess ... It's not my day  :(

>>>> I hope this helps (and doesn't start another argument).

we could bet whether

   static sig_atomic_t volatile stopFlag;

makes any difference to

  static bool stopFlag;

NB. for(;;) is generally more efficient for infinite loops -- less instructions...

while(false);
      0041138E  xor         eax,eax
      00411390  je          main+24h (411394h)
      00411392  jmp         main+1Eh (41138Eh)

do{}while(false);
      0041138E  xor         eax,eax
      00411390  jne         main+1Eh (41138Eh)

for(;;);
      0041138E  jmp         main+1Eh (41138Eh)

>> we could bet whether

If it solves the problem great, if it doesn't then so be it; however, ostensibly the type should be volatile.

The sig_atomin_t suggestion is a portability improvement.

I do not wish to argue about this again... we are both entitled to our opinion, I respect yours as I hope you do mine.

Regards,

-Rx.

>>>>  ... time to go to bed again I guess ...

I thought we live in the same time zone. It is 10 A.M. , isn't it?

NB. This suggestion for using volatile is, of course, predicated on the fact that the worker while loop is in a separate thread from the main process loop.

>>>> for(;;) is generally more efficient for infinite loops
That is not fair. You would need to compare with while(1) or while(true).

>> That is not fair. You would need to compare with while(1) or while(true).

You are right -- it was a typo... true condition is actually worse :-p

while(1);
0041138E  mov         eax,1
00411393  test        eax,eax
00411395  je          wmain+29h (411399h)
00411397  jmp         wmain+1Eh (41138Eh)

do {} while(1);
0041138E  mov         eax,1
00411393  test        eax,eax
00411395  jne         wmain+1Eh (41138Eh)

for(;;);
0041138E  jmp         wmain+1Eh (41138Eh)

...that said, a good industrial grade compiler should be able to optimize this away!

>> I thought we live in the same time zone. It is 10 A.M. , isn't it?

Yep ... Shows how bad I'm feeling ;)


>> NB. for(;;) is generally more efficient for infinite loops -- less instructions...

I have confidence in my compiler to sort out which is the most efficient way to implement a loop ... I use while (1) simply because I find it's easier to read.

>> ...that said, a good industrial grade compiler should be able to optimize this away!

Ah ... there you go heh.

>> Ah ... there you go heh
Operative word here is "should", which is not the same as "would"! When writing performance critical code (as I do habitually) it is better to write code the facilitates optimization, not hinder it! Of course if you don't care about performance then feel free to abuse your compiler in anyway you see fit -- it probably deserves it :)

>>>> if it doesn't then so be it;
No bet? Too bad.   ;-)

Seriously, the 'volatile' is a reasonable option (though not needed for nowadays compilers in case of multi-threading projects). The 'sig_atomic_t' is a (portable) type of less importance. At Intel, AMD (and all other modern) processors there is no need for it as all register operations were atomic. That's why it is typedef'd to an int. I never heard of that 'sig_atomic_t' before that thread (or don't remember) and I couldn't think that it has any minor or major advantage to using 'sig_atomic_t' instead of a 'bool' or 'int'.

>>>> feel free to abuse your compiler in anyway you see fit -- it probably deserves it :)
Wow. Big hammers for less than one nanosecond benefit.

>> I couldn't think that it has any minor or major advantage to using 'sig_atomic_t' instead of a 'bool' or 'int'.
Portability... it depends on how portable you want your code I guess :)

>> Wow. Big hammers for less than one nanosecond benefit.
It all adds up :)

>>>> it depends on how portable you want your code I guess
I respect your opinion but the statement that 'sig_atomic_t' is more portable than 'int' is nonsense. While any piece of source code was using 'int' type, the usage of 'sig_atomic_t' is rare exception. 'sig_atomic_t' has no relevance for nowadays compilers and you rarely will find code at the net which uses it (see below). Contrary, it signals a functionality that it doesn't have, i. e. that you could increment it in an atomic operation by different threads. But for that you need a mutex (or InterlockedIncrement at Windows OS). So, sig_atomic_t is simply misleading and I would recommend against using it, especially in a solution for beginners and students who can't judge the relevance of that.

Googling for 'sig_atomic_t'  gives 39,000 hits. The first pages show entries like

>> sig_atomic_t. Get answers to your questions in our C forum
>> you can use a particular data type for which access is always atomic: sig_atomic_t
>> But I'm not sure whether it is safe to implement a > mutex using sig_atomic_t
>> Since sig_atomic_t is defined as "typedef int > sig_atomic_t" for all arch's
>> Attached is a patch (against SAMBA_3_0) to use SIG_ATOMIC_T instead of sig_atomic_t.
>> explicit, we need to use "volatile sig_atomic_t". I see that

I didn't find one single code sample within the randomly viewed results. Or any new information beyond that we already discussed. Only questions, explanations, recommendations, non-recommendations ...

You say you respect my opinion but then preceded to be disrespectful in your response -- just because you don't personally use or see the need for something doesn't make it "nonsense"! I'm sure there are assertions you've made in this or other threads that I would equally consider nonsense but I wouldn't consider being so blunt or rude to a peer or fellow expert!

I never claimed int wasn't portable, on the contrary I specifically stated, "sig_atomin_t is a portable type (x-platform) that guarentees the type can be updated atomically (one CPU instruction). Although, in principle this should be true of any integer type using sig_atomin_t ensures you'll be using the correct type for the platform the code is built on."! What part of that don't you agree with?

In an attempt to appease your sensibilities; however, I will not raise the issue of usage of sig_atomic_t again during this thread. I hope this is acceptable to you and we can, hence-forth, have a harmonious co-existence!

-Rx.

>>> but then preceded to be disrespectful in your response
Sorry, no personal offense or even bluntness was intended.

The statement "it depends on how portable you want your code I guess" implies that an 'int' was less portable than a sig_atomic_t. I think that conclusion is wrong (not only a different opinion) and makes no sense. For the latter I usually say "nonsense" but it seems that I had to learn that the terms are not synonyms ... ;-)

>>>> and we can, hence-forth, have a harmonious co-existence!
Fully agreed. Looking forward to the next discussion.

>> Sorry, no personal offense or even bluntness was intended
Accepted, I'm sure there'll be no long term psychological scaring :)

>> implies that an 'int' was less portable than a sig_atomic_t
This is not what I meant to imply and I am sorry if the statement I made was in anyway misleading. NB. The standard says sig_atomic_t will be an integer type -- it doesn't state which one though :-p

>> For the latter I usually say "nonsense"
Maybe I took this out of context. Unfortunately, it is obviously hard to know without the benefit of body language, which is why I usually try to keep the semantics of my comments non-emotive or I'll provide an appropriate emoticon to help clarify the context.

>> Looking forward to the next discussion.
Me too :--D

-Rx.

Ok, whether I use an outer while loop with an inner flag, the thread doesn't toggle on/off. It will start, then turn I can turn it off, but after that it doesn't see the flag. Any further insight on this would be appreciated.

I have also tried the volatile approach as well, however I can't get my syntax correct to initialize the volatile member:

globals.h
=============
#include <csignal>
class Globals
{
public:
      static sig_atomic_t volatile stopFlag;
};

globals.cpp
============
#include "globals.h"
volatile Globals::stopFlag = 0; //<------------Error here, can't get this initialization right.

I am game for trying either approach...what ever works...

WLE

>> Error here, can't get this initialization right
Try: sig_atomic_t volatile Globals::stopFlag = 0;

This is interesting:

scenario 1 (change flag from another .cpp):
============
bool m_cond = true;
      while(m_cond){
        if(Globals::stopFlag)
        {
        MessageBox(NULL,"Thread On!","Error",MB_OK);
        wxThread::Sleep(1000);
        }
        else
        {
        MessageBox(NULL,"Thread Off!","Error",MB_OK);
        }
    }

MVMFrm.cpp
-----------------
(button push)
Flash8 = !Flash8;
      if (Flash8)
      {
      Globals::stopFlag = 1;
      MessageBox(NULL,"True!","Error",MB_OK);
    }
    else
    {
    Globals::stopFlag = 0;
    MessageBox(NULL,"False (off)!","Error",MB_OK);
    }

**Doesn't toggle back and forth **

scenario 2 (change flag from within thread)
============
bool m_cond = true;
      while(m_cond){
        if(Globals::stopFlag)
        {
        MessageBox(NULL,"Thread On!","Error",MB_OK);
        wxThread::Sleep(1000);
        Globals::stopFlag = 0;
        }
        else
        {
        MessageBox(NULL,"Thread Off!","Error",MB_OK);
        Globals::stopFlag = 1;
        }
    }

** toggles from within thread...

It appears that with or without the volatile statement changing the flag value is not being seen by the thread.

How can we make the thread see the flag change?

Let me get this right : in both scenario's the flag change is not seen ???

Have you tried using TRACE rather than MessageBox? I'm not suggesting this is the reason but trace will give you output without interupting the natural flow of both the main and the worker thread... allowing you to maybe spot what the problem is by look for anomolies in the trace output.

http://msdn2.microsoft.com/en-us/library/6w95a4ha(VS.80).aspx

Although this is not how I would write this code I can't see any obvious reason for the behavior you are seeing... although I have only given it a cursory glance as I am at work! I can look at it in more detail later at home... in the meantime can you post trace output and maybe put the full project as a zip on http://www.ee-stuff.com/ so we can build it ourselves and try debugging it (unless, of course, Alex or Infinity spot the problem first ;-> ).

-Rx.

>>>Let me get this right : in both scenario's the flag change is not seen ???

No, scenario 2 the flag change is seen.

Scenario 1, where the flag is changed from a different .cpp, is NOT seen.

Thanks WLE

Are you sure you're looking at the same flag ? Did you define the flag in the .h file ?

>>Did you define the flag in the .h file ?

I defined it in global.h...global.h is included in tracker_mod includes...does it need to be defined or externed in tracker_mod.h (the thread .h as well)?

It should be defined in a .cpp somewhere, so that there's only one in the whole application.

It is : global.cpp / global.h

Now if every time  I press my toggle button within 1 second of pressing ok on the message box, it toggles the thread correctly. This occurred after I put a sleep statement in the else condition (see below). So in a halarious twist, here I am pressing the toggle buttons making the thread work correctly as long as the flag is changed during the sleep interval (1000 ms). If the flag is changed outside this interval, the thread gets "lost: and nothing happens. Why is this?
tracker_mod.cpp
===============
 Globals GlobalInst;
bool m_cond = true;
      while(m_cond){
        if(Globals::stopFlag)
        {
        MessageBox(NULL,"Thread On!","Error",MB_OK);
        wxThread::Sleep(1000);
        Globals::stopFlag = 0;
        }
        else
        {
        MessageBox(NULL,"Thread Off!","Error",MB_OK);
        Globals::stopFlag = 1;
        }
    }

MVMFrm.cpp
==========
 extern Globals GlobalInst;
...
(button push)
Flash8 = !Flash8;
      if (Flash8)
      {
      Globals::stopFlag = 1;
      MessageBox(NULL,"True!","Error",MB_OK);
    }
    else
    {
    Globals::stopFlag = 0;
    MessageBox(NULL,"False (off)!","Error",MB_OK);
    }

Also, if I change my flag before 1000ms is done, will the thread hopefully reawaken before the 1000ms is up?

volatile Globals::stopFlag = 0; //<------------Error here, can't get this initialization right.

It is

 int Globals::stopFlag = 0;

The volatile (grrrrrrrrr)  belongs to the header (if you really think you need it)

(evilrix, the 'sig_atomic_t volatile' only confused !!!)

>>>> If the flag is changed outside this interval, the thread gets "lost:
>>>> and nothing happens. Why is this?
Hmmm, outputting message boxes in a infinite loop of a thread is somewhat dangerous. A message box is a modal dialog which would break execution of the thread (block it). If the message box hides behind some window you do see no action. Even if you release it by 'OK', the next box will follow immediately blocking the thread again. You should add some flags so that the message box was outputted only once after toggling with the button (as I gave a sample above). Or use OutputDebugString("in thread: on recognized") waht would print to the output window in Visual Studio if invoking the App in trhe Visual Studio Debugger. Or use a Sleep with some seconds so that you have time to see what happened.

>>> will the thread hopefully reawaken before the 1000ms
No, of course not.  A Sleep(1000) sleeps 1 second (at least). If you want an event driven awaking you need to create an event by calling CreateEvent and call WaitForSingleObject in your thread waiting for the event handle that you have to pass somehow from the main thread to the thread. You can use a timeout in the WaitForSingleObject, say 1 second. But don't use message boxes that need a click on Ok every second or earlier cause I assume you are not fast enough to click and check within 1 second ...

>> the 'sig_atomic_t volatile' only confused !!!
How would making it an int have made this clearer? Anyway, forget about sig_atomic_t - I thought we'd agreed to drop that (/me pokes Alex in the ribs) :-p

>> int Globals::stopFlag = 0;
you need to provide the volatile qualifier in the definition if the type was originally declared with it, otherwise the compiler thinks it's a redefinition!

VC2005
sig_atomic_t Globals::stopFlag = 0;
.\main.cpp(3) : error C2373: 'stopFlag' : redefinition; different type modifiers
        c:\documents and settings\ricky cormier\my documents\visual studio 2005\projects\testr\testr\main.hpp(5) : see declaration of 'stopFlag'

gcc-4.1.1
x.cpp:3: error: conflicting declaration 'sig_atomic_t Globals::stopFlag'
x.hpp:5: error: 'Globals::stopFlag' has a previous declaration as 'volatile sig_atomic_t Globals::stopFlag'
x.cpp:3: error: declaration of 'volatile sig_atomic_t Globals::stopFlag' outside of class is not definition

>> The volatile (grrrrrrrrr)  belongs to the header (if you really think you need it)
Now now, calm down; I am only going by past experience and well documented fact (well, I think it is anyway)!

>> outputting message boxes in a infinite loop of a thread is somewhat dangerous
Yup, that's why I suggested using TRACE

I've suggested the code be posted on EE Stuff so it can be 'played with'. Any objections to that?

>> Also, if I change my flag before 1000ms is done, will the thread hopefully reawaken before the 1000ms is up?

As Alex said : no. You need to choose your sleep time for the responsiveness you want. 1 second is usually a bit long for a sleep - you can easily get by with a 10th of that, or even less.

If you truly need immediate responsiveness (but for gui's that's rare), then the sleep approach won't work.

>>>> I thought we'd agreed to drop that
Sorry I relapsed ...

globals.h
=============
#ifndef GLOBALS_H
#define GLOBALS_H

class Globals
{
public:
      static int volatile stopFlag;
};

#endif


globals.cpp
============
#include "globals.h"
int volatile Globals::stopFlag = 0; //<------------

globals.cpp uses the same type and makes an assignment. Only the 'static' keyword can/must be dropped cause it needs to be defined only in the class definition.

>> The volatile (grrrrrrrrr)  belongs to the header (if you really think you need it)

Right, I have a little mystery...

Now I admit I wrote this code very (VERY) quickly (my wife is trying to make me go shopping and I am stalling) so I am prepared to accept it may contain a defect (or two -- no laughing if it does!); however, please try it for yourself and feel free to use int rather than sig_atomic_t if you so wish -- it'll make no difference!

#include <iostream>
#include <string>
#include <windows.h>
#include <csignal>

class Globals
{
public:
      static sig_atomic_t stopFlag;
};

sig_atomic_t Globals::stopFlag = 0;

DWORD WINAPI ThreadProc(LPVOID)
{
      while(Globals::stopFlag == 0);
      std::cout << "Done" << std::endl;
      return 0;
}

int main(){
      HANDLE h = CreateThread(NULL, NULL, ThreadProc, NULL, 0, NULL);

      if(!h) { throw -1; } // Just exit

      std::string s;

      std::cin >> s;
      Globals::stopFlag = 1;
      std::cin >> s;
}

This is using a standard, unmodified, VC2005 Express project: -

/O2 /GL /D "WIN32" /D "NDEBUG" /D "_CONSOLE" /D "SCL_SECURE_NO_WARNINGS" /D "_UNICODE" /D "UNICODE" /FD /EHsc /MD /Fo"Release\\" /Fd"Release\vc80.pdb" /W3 /nologo /c /Wp64 /Zi /TP /errorReport:prompt

With the volatile qualifier on stopFlag, in ***Release*** mode I get "Done" output after I type junk and press Enter in the console window, meaning the thread sees the signal to stop. If I then subsequently remove the volatile qualifier I never see "Done" output, suggesting the stop signal is not 'seen' across the thread boundy). Using the debugger to monitor running threads I see behavior consistant with both of these senarios. Given this, I would welcome thoughts on why this might be since the only difference is the introduction of the volatile qualifier!

NB. I have not tested this under gcc but I would be very surprised if I saw different behavior!

-Rx.

NB. For the OP -- this is NOT an example of how to code this -- it is just a bit of Q&D test code :)

I compiled your code on VC6 adding a return 0; to main and added 1 at the prompt. The 'Done' comes immediately.
I changed to VC7.1 and got the same. I compiled in Release mode. No problems 'not using' volatile. I currently don't have a VC8 compiler available and have a hyper-threading CPU only (not a real dual-core or dual-processor). That behavior is according my experiences in the last 12 or 13 years. I cannot prove now that VC8 (VS2005 Prof. in my case) has the same behavior. Or that a dual-core or dual-processor where the threads may run in different cores may cause something different. But I will do these tests at Friday when I am at home and where I have both VC8 and dual-core.

WLE, what is your current compiler and environment?

/O2 compiler option is optimizing speed. But I had the same option in VC7 release.

Can you try to change it nevertheless?

WLE, if there is (maybe only) a rare chance that the compiler is optimizing when accessing shared data you should use the 'volatile' keyword as it doesn't harm. The syntax is

   static type volatile variable;  

in the class definition (header) and

   type volatile classname::variable = initvalue;

in the cpp  where the '= initvalue' maybe must dropped for class types.

>> adding a return 0; to main
The omission of return 0 was a typo (I did say I was rushing) -- but it is not required by the C++ standard and will default to 0 (much to my dismay!).

>> what is your current compiler and environment?
Visual Studio Express 2005 + SP1
Single core AMD Athlon XP 2800+
OS is XPSP2.

>> /O2 compiler option is optimizing speed
/O2 (max speed) It does not work
/Ox (full) It does not work
/O1 (max size) It works
/Od (disabled) It works

These are all consistent with my expectations!

I have previously witnessed this same behavior with VC6.0, VC7.0, VC8.0 and gcc3.4 ---> gcc-4.1.x

>> I cannot prove now that VC8 (VS2005 Prof. in my case) has the same behavior
You think I would lie? I wouldn't provide the code if I wasn't 100% confident the behavior I am seeing is consistent -- would I?

>> rare chance that the compiler is optimizing when accessing shared data
It amuses me that you just won't accept that given aggressive optimization the compiler can (and will!) cache the value in a register or CPU cache! :)

-Rx.

>>>> you just won't accept that given aggressive optimization the compiler
>>>> can (and will!) cache the value in a register or CPU cache! :)

Sorry, I have been grown up with compilers where the issue was a big problem. But, and that was the difference between us, I had a very long experience with VC compilers which had no problems to recognize that shared data cannot be optimized in a loop condition.

>>>> You think I would lie?
No. But nevertheless I couldn't verify your results. With compilers where you tell that you 'have previously witnessed this same behavior'. What do you expect me to believe? My own experiences or non-verified results where I have no explanations for? Even if it turns out that VS2005 makes agressive optimization again (what is strange cause 'compatibility' has been some kind of holy cow for MS in the past) my experiences with older VC compilers were still valid and must not be thrown away because of that.

And there is another reason that makes me doubting. Last year I ported a huge application (more than 10,000 sources, 10 millions lines of code, MFC, STL, multi-threading) from VC6 to VC8. We didn't experience any of the problems you 'witnessed' and we really should have if there are no other parameters which bewared us from the issues. So you have to wait til Friday where I will find out whether your results were reproducable with VC8 or where we have to admit that we simply don't know why it doesn't work in your environment and works in mine.

In any case you successed in convincing me that I won't recommend against the volatile keyword anymore though I still am convinced that a clever compiler shouldn't be trapped by that. A loop with a while condition depending on a 'global' (shared) variable in a multi-threading project environment *must* not be optimized to an infinite loop. That simply is a 'bug'.  

>> That simply is a 'bug'.  
No, it's called aggressive optimization. It would be a bug if you has no way to prevent it -- you do: -
1. Disable optimization
2. Use the volatile keyword!

Did you read the article by Andrei Alexandrescu that I posted?

"The volatile keyword was devised to prevent compiler optimizations that might render code incorrect in the presence of certain asynchronous events. For example, if you declare a primitive variable as volatile, the compiler is not permitted to cache it in a register -- a common optimization that would be disastrous if that variable were shared among multiple threads. So the general rule is, if you have variables of primitive type that must be shared among multiple threads, declare those variables volatile."

Anyway, If you choose to ignore volatile and write code without it; that's fine, I won't lose sleep -- promise!

I think I'm bored of this whole thread now... I won't be posting to it again, feel free to flame me all you like -- I'm off to help some people :-p

-Rx (over and out)

Ok, I got it working BUT I have come this far only to find out the cost Sleep is too high as far as responsiveness. I got the while loop going with the flag (fixed some other interfering problems) and got the processor commitment down to 15-24% with Sleep (30). Well, 30ms is way too high as my camera is 80FPS. The sleep bottlenecks me to less than 30. So I think we ought to either do some sort of event driven thread stuff or create and run the thread everytime...I would like to explore the former.

>>If you truly need immediate responsiveness (but for gui's that's rare), then the sleep approach won't work.

Ok, I went around the barn to do this, now what do I do?

Wait a sec ... if you have to process 80 fps, then why are we even talking about sleeping ? Why do we even need threads ? A simple loop receiving a picture and processing it should be sufficient, no ?

If you do need a separate thread, then you just keep a buffer with incoming pictures that still need to be processed, and a semaphore indicating how many are available for processing.

>>Wait a sec ... if you have to process 80 fps, then why are we even talking about sleeping ?

Exactly. Just my inexperience I guess. It took me going through this process to really understand it. Sorry.

>>A simple loop receiving a picture and processing it should be sufficient, no ?
                                            (time) +        (more time)

Hence the meat of this post. My awesome frame rate of 83+ will suffer if I also machine vision process the image obtained *before* I get the next one. Therefor it only makes sense to obtain the image and restart image acquisition loop while the last image is undergoing machine vision analysis. Camera API costs...Like evilrx said, these little costs alll add up.

you would have me do this?

FrameCallBack {

...obtain image
...process image

}



I would rather do some parallel processing:

FrameCallBack
{
...obtain image--------------------------------->Process image
}
( go back to frame callback AS image is being processed)


So I need to divide the loop...problem is, if I do that with a function call, the loop stops until the process image returns a value, thus prompting my perceived need for threads.

So whats the best way to do divide the work?

That is...when the framecallback acquires the image, it is released to go retrieve another image without first waiting on analysis. So folks please help me with a strategy to divide the work as shown above....any simple ideas I might have overlooked?

WLE

>> If I am right you already do it the way Infinity suggested.

Except that it would be a buffer that can contain more than one image ...

>>Didn't you already read the image in mvrmfrm.cpp

No that is GUI control. MachineVisionMod.cpp has the FrameCallBack function which obtains and buffers the image in an IPLImage format ready for OpenCV processing. Tracker_Mod.cpp thread was triggered after the image is set, allowing FrameCallBack to restart while MachineVision processing goes about. The ony API I/O is during image aquisition in MachineVisionMod.cpp (the name is a misnomer as the machie vision portion is really done else where) - it should be renamed camera_api.cpp or something like that for clairty).

>>a buffered approach would be nicer, where the first thread reads the images,

I'm all ears!

So,

MVMFrm.cpp ------->GUI (toggle control)
MachineVisionMod.cpp-------------->camera image acquisition (we should rename the file)
tracker_mod.cpp (thread)----------->machine vision processing triggered by MachineVisionMod.cpp

Thanks WLE

I would like to know if you have (estimations of) values for x1, y1, x2 and y2 (see my earlier post). It will allow to really present you with a good solution.


>> I'm all ears!

There's not much more to it than I already said : if a separate thread for analyzing increases performance, then you can make it better by not just buffering one image, but making room for multiple images in the buffer. That way the analysis thread will waste as little time as possible sleeping, because it can continue as long as there are images available in the buffer. The Sleep(0) "trick" Alex mentioned will also work good in combination with this.

>>I would like to know if you have (estimations of) values for x1, y1, x2 and y2 (

Will have to check the API mats from manufacturer. Will post. Can you suggest any ways of measuring routines (maybe a tick counter)?

>>then you can make it better by not just buffering one image, but making room for multiple images in the buffer.

And how would this be done?

>> Can you suggest any ways of measuring routines (maybe a tick counter)?

You don't need exact values - just estimates (how much I/O do both operations do ?). Measure the time it actually takes, and put it against the time it should take if there's no waiting for I/O.
Just rough measures are ok :

1a) how long does it take to acquire one image (start to end)
1b) what percentage of that time was it waiting for I/O ?
2a) how long does it take to analyze one image (start to end)
2b) what percentage of that time was it waiting for I/O ?


>> And how would this be done?

Create a buffer that can hold multiple images, and keep a semaphore next to it that holds the amount of images in the buffer that are ready to be processed. As long as that counter doesn't hit 0, the analyzing thread can do its work. Of course the semaphore gets incremented everytime an image is read and put in the buffer.

        http://en.wikipedia.org/wiki/Semaphore_(programming)

It's not very different from what we discussed earlier with the mutex, apart from the fact that the buffer is larger of course.

Again : the first question is still whether there would be a gain in using threads.