What's the best approach to achieve uniqueness in an object shared by multiple threads?

Question

I am interested in timing my function calls to database + other functions to build some metrics for my application's performance. I used Stopwatch and a metrics object, but it doesn't seem to consistently give correct values. Sometimes the elapsed time for calling a function is exactly the same for all the calls which is unrealistic...

I have discovered that the reason for the problem is due to the Metrics object properties values. The values of one Metrics object get overwritten when other instances of the Metrics generated by other threads are assigned values. It seems like the properties values are per reference although a new instance is created by each thread.

What's the best approach to achieve uniqueness in an object shared by multiple threads?

Code below:

private Metrics Metrics;
private Stopwatch Stopwatch;
private int DegreeOfParallelism { get { return Convert.ToInt32(ConfigurationManager.AppSettings["DegreeOfParallelism"].ToString()); } }

var lOptions = new ParallelOptions() { MaxDegreeOfParallelism = DegreeOfParallelism };
Parallel.ForEach(RequestBag, lOptions, (lItem, loopState) =>
{
    if (!string.IsNullOrEmpty(lItem.XmlRequest))
    {
        try
        {
            Metrics = new Metrics();
            Stopwatch = new Stopwatch();
            Stopwatch.Start();
            ObjRef = new Object();
            lItem.XmlRequest = ObjRef.GetDecision(Username, Password);
            Stopwatch.Stop();
            Metrics.ElapsedTime = string.Format("{0:0.00}", Stopwatch.Elapsed.TotalSeconds);

            Stopwatch.Restart();
            if (!string.IsNullOrEmpty(DBConnectionString))
            {
                DataAccess = new DataAccess2(DBConnectionString);
                DataAccess.WriteToDB(lItem.XmlRequest);  
            }
            Stopwatch.Stop();
            Metrics.DbFuncCallTime = string.Format("{0:0.00}", Stopwatch.Elapsed.TotalSeconds); 
        }
        catch (Exception pEx)
        { 
            KeepLog(pEx);
            Metrics.HasFailed = true;
        }
        finally
        {
            ProcessedIdsBag.Add(lItem.OrderId);
            Metrics.ProcessedOrderId = lItem.OrderId;
            Metrics.DegreeOfParallelism = DegreeOfParallelism;
            Metrics.TotalNumOfOrders = NumberOfOrders;
            Metrics.TotalNumOfOrdersProcessed = ProcessedIdsBag.Count;
            pBackgroundWorker.ReportProgress(Metrics.GetProgressPercentage(NumberOfOrders, ProcessedIdsBag.Count), Metrics);

            RequestBag.TryTake(out lItem);
        }
    }
});

Any help will be very much appreciated. Thanks, R

Answer 1

What it seems that you want to do is create a metrics object for each iteration, and then aggregate them at the end:

private ConcurrentBag<Metrics> allMetrics = new ConcurrentBag<Metrics>();
private int DegreeOfParallelism { get { return Convert.ToInt32(ConfigurationManager.AppSettings["DegreeOfParallelism"].ToString()); } }

var lOptions = new ParallelOptions() { MaxDegreeOfParallelism = DegreeOfParallelism };
Parallel.ForEach(RequestBag, lOptions, (lItem, loopState) =>
{
    if (!string.IsNullOrEmpty(lItem.XmlRequest))
    {
        try
        {
            var Metrics = new Metrics();
            var Stopwatch = new Stopwatch();
            Stopwatch.Start();
            ObjRef = new Object();
            lItem.XmlRequest = ObjRef.GetDecision(Username, Password);
            Stopwatch.Stop();
            Metrics.ElapsedTime = string.Format("{0:0.00}", Stopwatch.Elapsed.TotalSeconds);

            Stopwatch.Restart();
            if (!string.IsNullOrEmpty(DBConnectionString))
            {
                DataAccess = new DataAccess2(DBConnectionString);
                DataAccess.WriteToDB(lItem.XmlRequest);  
            }
            Stopwatch.Stop();
            Metrics.DbFuncCallTime = string.Format("{0:0.00}", Stopwatch.Elapsed.TotalSeconds); 
        }
        catch (Exception pEx)
        { 
            KeepLog(pEx);
            Metrics.HasFailed = true;
        }
        finally
        {
            ProcessedIdsBag.Add(lItem.OrderId);
            Metrics.ProcessedOrderId = lItem.OrderId;
            Metrics.DegreeOfParallelism = DegreeOfParallelism;
            Metrics.TotalNumOfOrders = NumberOfOrders;
            Metrics.TotalNumOfOrdersProcessed = ProcessedIdsBag.Count;
            pBackgroundWorker.ReportProgress(Metrics.GetProgressPercentage(NumberOfOrders, ProcessedIdsBag.Count), Metrics);

            RequestBag.TryTake(out lItem);
            allMetrics.add(Metrics);
        }
    }
});

// Aggregate everything in AllMetrics here

Answer 2

You need to change the scope of your Stopwatch and Metrics variables.

Currently, each thread shares the same Metrics variable. As soon as a thread enters the try block, it creates a new instance of Metrics (correctly), but puts it a shared variable (incorrectly). All other threads will see that new instance when reading the shared variable until the next thread comes along and start the whole process over.

move

private Metrics Metrics;
private Stopwatch Stopwatch;

to just inside your loop

Parallel.ForEach(RequestBag, lOptions, (lItem, loopState) =>
{
    private Metrics Metrics;
    private Stopwatch Stopwatch;
...

This will give each iteration through the loop it's own variable in which to store it's own instance of the the object.