mixpanel/mixpanel-unity

Worker thread uses dangerous pattern

thegoldenmule opened this issue · 3 comments

We've noticed that the thread created in Worker.cs is using a dangerous consumer pattern relying on spin in a while loop. This drains battery and uses unnecessary resources. Here is the relevant blurb:

private static void RunBackgroundThread()
{
    _isBgThreadRunning = true;
    while (!_stopThread)
    {
        try
        {
            DispatchOperations();
        }
        catch (Exception e)
        {
            Mixpanel.LogError(e.ToString());
        }  
    }
    _isBgThreadRunning = false;
}


private static void DispatchOperations()
{
    if (_ops.Count == 0) return;

    // elided
}

This is a tight loop that executes as quickly as it can, 99% of application uptime (when there is no op to consume). This is a dangerous pattern and consumes far more resources than it needs. This consumes so many resources that we have opted to manually send HTTP calls instead of using this library.

Instead, it's recommended to use a more standard pattern like Monitor.Wait and Monitor.Pulse. Using this pattern the consumer thread does not spin, it blocks until there is something to consume, and only then does the work.

Thanks @thegoldenmule for bringing this up. It has been on the back of my head for a while now, and I went ahead and implement what you suggested. I appreciate you opening this issue. Feel free to look at the PR: #99

Thanks for the update!

Available in the latest release v2.1.4. Closing this issue