views:

262

answers:

6

My program does file loading and memcpy'ing in the background while the screen is meant to be updated interactively. The idea is to have async loading of files the program will soon need so that they are ready to be used when the main thread needs them. However, the loads/copies don't seem to happen in parallel with the main thread. The main thread pauses during the loading and will often wait for all loads (can be up to 8 at once) to finish before the next iteration of the main thread's main loop.

I'm using Win32, so I'm using _beginthread for creating the file-loading/copying thread.

The worker thread function:

void fileLoadThreadFunc(void *arglist)
{
    while(true)
    {
     // s_mutex keeps the list from being updated by the main thread
     s_mutex.lock();  // uses WaitForSingleObject INFINITE
     // s_filesToLoad is a list added to from the main thread
     while (s_filesToLoad.size() == 0)
     {
         s_mutex.unlock();
         Sleep(10);
         s_mutex.lock();
     }
     loadObj *obj = s_filesToLoad[0];
     s_filesToLoad.erase(s_filesToLoad.begin());
     s_mutex.unlock();

     obj->loadFileAndMemcpy();
    }
}

main thread startup:

_beginThread(fileLoadThreadFunc, 0, NULL);

code in a class that the main thread uses to "kick" the thread for loading a file:

// I used the commented code to see if main thread was ever blocking
// but the PRINT never printed, so it looks like it never was waiting on the worker
//while(!s_mutex.lock(false))
//{
//  PRINT(L"blocked! ");
//}
s_mutex.lock();
s_filesToLoad.push_back(this);
s_mutex.unlock();

Some more notes based on comments:

  • The loadFileAndMemcpy() function in the worker thread loads via Win32 ReadFile function - does this cause the main thread to block?
  • I reduced the worker thread priority to either THREAD_PRIORITY_BELOW_NORMAL and THREAD_PRIORITY_LOWEST, and that helps a bit, but when I move the mouse around to see how slowly it moves while the worker thread is working, the mouse "jumps" a bit (without lowering the priority, it was MUCH worse).
  • I am running on a Core 2 Duo, so I wouldn't expect to see any mouse lag at all.
  • Mutex code doesn't seem to be an issue since the "blocked!" never printed in my test code above.
  • I bumped the sleep up to 100ms, but even 1000ms doesn't seem to help as far as the mouse lag goes.
  • Data being loaded is tiny - 20k .png images (but they are 2048x2048).. they are small size since this is just test data, one single color in the image, so real data will be much larger.
A: 

Not sure on your specific problem but you really should mutex-protect the size call as well.

void fileLoadThreadFunc(void *arglist) {
    while (true) {
        s_mutex.lock();
        while (s_filesToLoad.size() == 0) {
            s_mutex.unlock();
            Sleep(10);
            s_mutex.lock();
        }
        loadObj *obj = s_filesToLoad[0];
        s_filesToLoad.erase(s_filesToLoad.begin());
        s_mutex.unlock();
        obj->loadFileAndMemcpy();
    }
}

Now, examining your specific problem, I can see nothing wrong with the code you've provided. The main thread and file loader thread should quite happily run side-by-side if that mutex is the only contention between them.

I say that because there may be other points of contention, such as in the standard library, that your sample code doesn't show.

paxdiablo
Updated my OP since it's not obvious from the code that I wasn't using a mutex due to a custom container implementation that just returns an int member variable.
Jim Buck
@Jim, I wasn't suggesting that *specific* mutex was causing main thread to block. There are other points where it can block such as (possibly) in file I/O routines if they have a global lock on them.
paxdiablo
Just out of interest, what happens if you bump the sleep call up from 10ms to say 1000ms?
paxdiablo
In addition, I'd be looking at what goes on in obj->loadFileAndMemcpy() that could possibly block the main thread from running. The reason I ask is because you state "will *often* wait for all loads" (not *always*) which could indicate a problem there.
paxdiablo
A: 

I'd write that loop this way, less locking unlock which could get messed up :P :

void fileLoadThreadFunc(void *arglist)
{
    while(true)
    {
     loadObj *obj = NULL;

        // protect all access to the vector
        s_mutex.lock();
        if(s_filesToLoad.size() != 0)
        {
             obj = s_filesToLoad[0];
             s_filesToLoad.erase(s_filesToLoad.begin());
        }
 s_mutex.unlock();

 if( obj != NULL )
         obj->loadFileAndMemcpy();
        else
            Sleep(10);

    }
}

MSDN on Synchronization

sfossen
Why *always* Sleep for 10mSec. sleep only when the queue is empty
aJ
A: 

if you can consider open source options, Java has a blocking queue [link] as does Python [link]. This would reduce your code to (queue here is bound to load_fcn, i.e. using a closure)

def load_fcn():
    while True:
        queue.get().loadFileAndMemcpy()
threading.Thread(target=load_fcn).start()

Even though you're maybe not supposed to use them, python 3.0 threads have a _stop() function and python2.0 threads have a _Thread__stop function. You could also write a "None" value to the queue and check in load_fcn().

Also, search stackoverflow for "[python] gui" and "[subjective] [gui] [java]" if you wish.

gatoatigrado
+1  A: 

You will have to show the code for the main thread to indicate how it is notified that it a file is loaded. Most likely the blocking issue is there. This is really a good case for using asynchronous I/O instead of threads if you can work it into your main loop. If nothing else you really need to use conditions or events. One to trigger the file reader thread that there is work to do, and another to signal the main thread a file has been loaded.

Edit: Alright, so this is a game, and you're polling to see if the file is done loading as part of the rendering loop. Here's what I would try: use ReadFileEx to initiate an overlapped read. This won't block. Then in your main loop you can check if the read is done by using one of the Wait functions with a zero timeout. This won't block either.

karunski
The main thread isn't really notified of the file loading being done. They are sprite graphics that are being loaded, and the sprite object doesn't render itself if the texture object's bLoaded flag is false. bLoaded is set by the loading thread. How to do async loading by not using a thread?
Jim Buck
You would use ReadFileEx to start an overlapped I/O operation. See the edit.
karunski
A: 

Based on the information present at this point, my guess would be that something in handler for the file loading is interacting with your main loop. I do not know the libraries involved, but based on your description the file handler does something along the following lines:

  • Load raw binary data for a 20k file
  • Interpret the 20k as a PNG file
  • Load into a structure representing a 2048x2048 pixel image

The following possibilities come to mind regarding the libraries you use to achieve these steps:

  • Could it be that the memory allocation for the uncompressed image data is holding a lock that the main thread needs for any drawing / interactive operations it performs?
  • Could it be that a call that is responsible for translating the PNG data into pixels actually holds a low-level game library lock that adversely interacts with your main thread?

The best way to get some more information would be to try and model the activity of your file loader handler without using the current code in it... write a routine yourself that allocates the right size of memory block and performs some processing that turns 20k of source data into a structure the size of the target block... then add further logic to it one bit at a time until you narrow down when performance crashes to isolate the culprit.

jerryjvl
A: 

I think that your problem lies with access to the FilesToLoad object.

As I see it this object is locked by your thread when the it is actually processing it (which is every 10ms according to your code) and by your main thread as it is trying to update the list. This probably means that your main thread is waiting for a while to access it and/or the OS sorts out any race situations that may occur.

I would would suggest that you either start up a worker thread just to load a file when you as you want it, setting a semaphore (or even a bool value) to show when it has completed or use _beginthreadex and create a suspended thread for each file and then synchronise them so that as each one completes the next in line is resumed.

If you want a thread to run permenently in the background erasing and loading files then you could always have it process it's own message queue and use windows messaging to pass data back and forth. This saves a lot of heartache regarding thread locking and race condition avoidance.

ChrisBD