Delayed function call?

Hello all,

This is a bit of a noob question, but when it comes to threading I think I’m a perpetual noob…
I need something to call a function, but in a delayed way (delayed x miliseconds). The function should be called in a thread that can take some time without blocking other things.
Right now I’m using the ThreadPool to call the functions, but I want to delay the actual function calls, and as far as I know I shouldn’t be blocking the threads in the ThreadPool - right??.
Browsing through the codebase I looked at Timer, WaitableObject, the Message stuff and I just get more confused. Especially Timer seems quite complex! Any help would be really welcome.

  • bram

If you don’t need a specific time you can have a look at AsyncUpdater. Also, Timer is rather easy to implement:

class MyClass : public Timer
{
public:
  void someFunction()
  {
    // [...]
    if (somethingToDoInTheFuture)
      startTimer(100); // do Job in 100 ms
  }

  void timerCallback()
  {
    // do something
    stopTimer();
  }
};

I read somewhere here that the first timer callback might come earlier than the time given. Don’t know if this is still the case, but if it is a problem you can use a smaller timer Intervall and save a start time (Time::getMillisecondCounter()) and check whether a given time elapsed.

Chris

I don’t think that was ever the case… Timers can get delayed by other activity on the message thread, so they can sometimes fire too late, but I don’t think they’ll ever fire early, at least not by a significant interval.

Hmm, okay, that seems like a great idea, but then the cleanup is still missing:
I would like to be able to create a new call object and have it destroy itself when it’s done.

I.e. something like

class MyFunction : public DelayedFunctionCall
{
   virtual void Call()
   {
      // do something here
   }
}

new MyFunction(); // will get cleaned up automatically.... :-/
  • bram

why not putting the actual wait in the function or another function that wrap this one as this is called in another thread ?

Hey Olivier,

Well, tbh I was wondering if this couldn’t be done with the ThreadJobPool… I.e. just do a wait in the runJob of a ThreadPoolJob?
The advantage there would be that I don;t have to do the cleanup, the ThreadPool already does it for me…

  • bram

But then again doing a wait() in a threadpooljob might be trouble?
Something similar here: http://www.rawmaterialsoftware.com/viewtopic.php?f=2&t=7311

  • bram

In your case just add juce::Thread::sleep(10000)
at the beginning of the runJob function

There is no reason that it shouldn’t work.
and you can even check at the end of the wait if the operation still needs to be done.(allowing cancel)

(A nicer approach would be indeed to use your own WaitableEvent stored in the job with a timeout to allow earlier cancel and not block the job of the whole timeout)

Adding the sleep causes all kinds of “still running threads” problems when exiting though…

  • bram

Then use a WaitableEvent that you cancel in the dtor.

[quote=“jules”]
I don’t think that was ever the case… Timers can get delayed by other activity on the message thread, so they can sometimes fire too late, but I don’t think they’ll ever fire early, at least not by a significant interval.[/quote]
You’re right. I think I confused it with this thread.

Chris

I’m going to start trying with a completely different way, but if someone ever finds a way to fix this in a SIMPLE way, let me know:

    class DelayedThreadPoolJob : public ThreadPoolJob
    {
    public:
        DelayedThreadPoolJob(const String& name, unsigned long delayInMs) :
            ThreadPoolJob(name),
            m_DelayInMs(delayInMs)
        {
        }

        ~DelayedThreadPoolJob()
        {
            m_WaitableEvent.signal();
        }

        virtual ThreadPoolJob::JobStatus runDelayedJob() = 0;
    private:
        friend class ThreadPool;

        virtual ThreadPoolJob::JobStatus runJob()
        {
            bool wasSignalled = m_WaitableEvent.wait(m_DelayInMs);
            if (wasSignalled)
                return ThreadPoolJob::jobHasFinished;
            return runDelayedJob();
        }

        WaitableEvent m_WaitableEvent;
        unsigned long m_DelayInMs;
    };

Your job could just spin, e.g.

[code]void runJob()
{
for (int i = 1000; --i >= 0;)
{
Thread::sleep (10);

    if (shouldExit())
        return;
}

[/code]

…not elegant, but would avoid messing about with events. I don’t think your code would work properly anyway, as the destructor won’t be called until the job has already finished.

Personally, I’d kick off a timer that would launch a thread to do the work at the right time, rather than having a thread sitting there waiting.

Hmm, okay… what do you guys think about this?

The only thing I can think about is a possible mess when the destruction happens…

    class TimedThreadPool : private Timer, private ThreadPool
    {
    public:
        class TimedThreadPoolJob : public ThreadPoolJob
        {
        public:
            TimedThreadPoolJob(uint32 delayInMs, const String& name) :
                ThreadPoolJob(name),
                m_DelayInMs(delayInMs)
            {

            }

            uint32 GetWaitTime() { return m_DelayInMs; }
        private:
            uint32 m_DelayInMs;
        };

        TimedThreadPool() :
            m_AbstractFifo(c_Capacity),
            m_CurrentJob(nullptr)
        {
            startTimer(100);
        };

        virtual ~TimedThreadPool()
        {
            stopTimer();

            TimedThreadPoolJob* job;
            
            while (ReadFromFifo(&job))
                delete job;

            if (m_CurrentJob)
                delete m_CurrentJob;
        };

        bool AddToQueue(TimedThreadPoolJob* job)
        {
            int start1, size1, start2, size2;
            m_AbstractFifo.prepareToWrite (1, start1, size1, start2, size2);
            bool wasWritten = size1 + size2 != 0;
            
            if (size1 > 0)
                m_CheckJobs[start1] = job;
            
            if (size2 > 0)
                m_CheckJobs[start2] = job;
            
            m_AbstractFifo.finishedWrite(size1 + size2);

            return wasWritten;
        }

    protected:
        virtual void timerCallback()
        {
            if (m_CurrentJob)
            {
                unsigned long waitTime = m_CurrentJob->GetWaitTime();
                uint32 currentTime = Time::getMillisecondCounter();
                uint32 timeElapsed = currentTime >= m_StartTime ? currentTime - m_StartTime : (std::numeric_limits<uint32>::max() - m_StartTime) + currentTime;
                if (timeElapsed > waitTime)
                {
                    addJob(m_CurrentJob, true);
                    m_CurrentJob = nullptr;
                }
            }
            else
            {
                if (ReadFromFifo(&m_CurrentJob))
                {
                    m_StartTime = Time::getMillisecondCounter();
                }
                else
                {
                    m_CurrentJob = nullptr;
                }
            }
        }

    private:
        bool ReadFromFifo(TimedThreadPoolJob** job)
        {
            int start1, size1, start2, size2;
            m_AbstractFifo.prepareToRead(1, start1, size1, start2, size2);
            bool wasRead = size1 + size2 != 0;

            if (size1 > 0)
                *job = m_CheckJobs[start1];

            if (size2 > 0)
                *job = m_CheckJobs[start2];

            m_AbstractFifo.finishedRead(size1 + size2);

            return wasRead;
        }

        const static int c_Capacity = 1024;
        AbstractFifo m_AbstractFifo;
        TimedThreadPoolJob* m_CheckJobs[c_Capacity];
        TimedThreadPoolJob* m_CurrentJob;
        uint32 m_StartTime;
        
        // Prevent default copy constructor and assignment operator from being generated.
        TimedThreadPool(const TimedThreadPool&);
        TimedThreadPool& operator = (const TimedThreadPool&);
    };

I saw a couple of deletes in there, and stopped reading at that point. It’s 2012, nobody should still be using raw deletes in modern code!

Then how does one clean up an AbstractFifo without using delete?

while (true)
        {
            ScopedPointer<TimedThreadPoolJob> job;

            if (!ReadFromFifo(job))
                break;
        }

feels slightly ridiculous…

  • Bram

What is “job”? Is it returned by that function? Who owns it? Who created it? Whose responsibility is it to delete it? It’s not obvious to me what’s going on in your code there.

RAII is only difficult to use when you have objects whose ownership is tangled or unclear. And in my humble experience, whenever you have code like that, it’s time for a good re-think!

I rewrote it without the Fifo and just and OwnedArray (with Critical Section), … it’s a lot simpler now, I don’t know why I always want to use lockfree structures :wink: Maybe you can have a look at this one and not feel disgusted :wink:


    class TimedThreadPool : private Timer, private ThreadPool
    {
    public:
        class TimedThreadPoolJob : public ThreadPoolJob
        {
        public:
            TimedThreadPoolJob(uint32 delayInMs, const String& name);
            uint32 GetWaitTime();
        private:
            uint32 m_DelayInMs;
        };

        TimedThreadPool();;

        virtual ~TimedThreadPool();;

        void AddToQueue(TimedThreadPoolJob* job);

    protected:
        virtual void timerCallback();

    private:
        bool ReadFromFifo(ScopedPointer<TimedThreadPoolJob>& job);

        OwnedArray<TimedThreadPoolJob, CriticalSection> m_CheckJobs;
        ScopedPointer<TimedThreadPoolJob> m_CurrentJob;
        uint32 m_StartTime;
        
        // Prevent default copy constructor and assignment operator from being generated.
        TimedThreadPool(const TimedThreadPool&);
        TimedThreadPool& operator = (const TimedThreadPool&);
    };

    TimedThreadPool::TimedThreadPoolJob::TimedThreadPoolJob(uint32 delayInMs, const String& name) :
        ThreadPoolJob(name),
        m_DelayInMs(delayInMs)
    {

    }

    uint32 TimedThreadPool::TimedThreadPoolJob::GetWaitTime()
    {
        return m_DelayInMs;
    }

    TimedThreadPool::TimedThreadPool()
    {
        startTimer(100);
    }

    TimedThreadPool::~TimedThreadPool()
    {
        stopTimer();
    }

    void TimedThreadPool::AddToQueue(TimedThreadPoolJob* job)
    {
        m_CheckJobs.add(job);
    }

    void TimedThreadPool::timerCallback()
    {
        if (m_CurrentJob.get() != nullptr)
        {
            unsigned long waitTime = m_CurrentJob->GetWaitTime();
            uint32 currentTime = Time::getMillisecondCounter();
            uint32 timeElapsed = currentTime >= m_StartTime ? currentTime - m_StartTime : (std::numeric_limits<uint32>::max() - m_StartTime) + currentTime;
            if (timeElapsed > waitTime)
                addJob(m_CurrentJob.release(), true);
        }
        else
        {
            if (m_CheckJobs.size())
            {
                m_CurrentJob = m_CheckJobs.removeAndReturn(0);
                m_StartTime = Time::getMillisecondCounter();
            }
        }
    }
  • bram

Better, but you’ve still committed another dreadful coding sin: Using ThreadPool as a base class!

Every day before breakfast, repeat to yourself 100 times: “Composition is better than inheritance”!

[quote=“jules”]
RAII is only difficult to use when you have objects whose ownership is tangled or unclear.[/quote]

Or very large objects.