在并行foreach循环执行期间，变量的值会互换 [英] Values of Variables get interchanged during the execution of parallel foreach loop

问题描述

我正在使用并行的foreach循环，如下所示

在上面的

Hi,

I am using Parallel foreach loop like following

Parallel.ForEach(dt.AsEnumerable(), dr =>
            {
                object state = dr;
                var task = Task.Factory.StartNew(() => DoSomeWork(state), TaskCreationOptions.PreferFairness);
                tasks[ctr] = task;
                ctr++;
                
            });

中，对于每个循环，我正在使用一个名为DoSomeWork()的函数，函数的详细信息如下:

in the above Parallel for each loop i am using a funtion namely DoSomeWork() the details of function is as follows:

void DoSomeWork(object state)
   {
       lock (lockThis)
       {
           DataRow dr = (DataRow)state;
           string uri = dr["PingURL"].ToString();
           string BuyerId = dr["BuyerId"].ToString();
           string PostT = dr["PostType"].ToString();
           string ResponseT = dr["ResponseType"].ToString();
           FirstTask(xmlData, uri, BuyerId, PostT, ResponseT);
       }
   }

在上面的函数中，使用了另一个函数，即FirstTask()，其中我正在使用全局变量进行数据库操作.
但是变量的值在执行期间会互换.功能的详细信息如下:

in the above function another function is used namely FirstTask() in which i am doing the database operation using global variables.
But the values of the variable gets interchanged during the execution. The details of function is as follow:

void FirstTask(string i, string uri, string buyerid, string PostT, string ResponseT)
    {
        System.Net.WebRequest req = null;
        System.Net.WebResponse rsp = null;
        try
        {
            string rp = "";
            //Pinging Code
            if (PostT == "XML Post")
            {
                rp = XMLPostType(req, uri, buyerid, rsp);
            }
            else if (PostT == "Form Post")
            {
                rp = FormPostType(uri, buyerid);
            }
            //Return if no response
            if (rp == "" || rp == "return")
            {
                return;
            }
            //parse Response
            if (ResponseT == "XML Response")
            {
                XMLResponseType(rp, buyerid);
            }
            else if (ResponseT == "Form Response")
            {
                FormResponseType(rp, buyerid);
            }

            DataClassesDataContext DCDCU = new DataClassesDataContext();
            var query = from BRRD in DCDCU.BuyerRequestResponseDetails
                        where BRRD.LeadId == Convert.ToInt64(v.LLeadID) && BRRD.BuyerId == Convert.ToInt64(buyerid)
                        select BRRD;
            foreach (BuyerRequestResponseDetail br in query)
            {
                br.PingResponse = rp;
            }
            DCDCU.SubmitChanges();

            DataClassesDataContext DCDC = new DataClassesDataContext();

            LeadResponse lr = new LeadResponse()
            {
                BuyerId = Convert.ToInt64(buyerid),
                BuyerRefNo = rv.RBuyerRefNO,
                BuyerPrice = rv.RBuyerPrice,
                BuyerStatus = rv.RBuyerStatus,
                BuyerMessage = rv.RBuyerMessage,
                LeadId = Convert.ToInt64(v.LLeadID),
                ResponseDateTime = DateTime.Now
            };

            DCDC.LeadResponses.InsertOnSubmit(lr);
            if (dtProfit.Rows.Count > 0)
            {
                double PriceAP = 0;
                if (dtProfit.Rows[0]["ProfitType"].ToString() == "Fixed")
                {
                    lr.PriceAfterProfit = System.Math.Round((Convert.ToDouble(lr.BuyerPrice) - Convert.ToDouble(dtProfit.Rows[0]["ProfitValue"].ToString())), 2);
                }
                else
                {
                    lr.PriceAfterProfit = System.Math.Round(Convert.ToDouble(lr.BuyerPrice) - ((Convert.ToDouble(lr.BuyerPrice) * Convert.ToDouble(dtProfit.Rows[0]["ProfitValue"].ToString())) / 100), 2);
                }
                PriceAP = Convert.ToDouble(lr.PriceAfterProfit);
                lr.ProfitId = Convert.ToInt64(dtProfit.Rows[0]["ProfitId"].ToString());

            }
            DCDC.SubmitChanges();
            rv.RBuyerRefNO = "";
            rv.RBuyerPrice = 0.0;
            rv.RBuyerStatus = "";
            rv.RBuyerMessage = "";
        }
        catch (System.Net.WebException)
        {
        }
        catch (Exception)
        {
        }
        finally
        {
            if (req != null) req.GetRequestStream().Close();
            if (rsp != null) rsp.GetResponseStream().Close();
        }
    }

请帮帮我，

在此先感谢

Please help me,

Thanks in advance

推荐答案

很难深入了解所有细节，但是即使乍一看，我仍然可以告诉您正在尝试滥用技术.看起来您正在尝试通过越多越好"的原则来坚持并行执行.事情不是这样工作的.

您描述的负面影响称为竞争状况.当您说要交换变量时，您不必解释哪个变量(这是问题所在，请参阅下文)，但是您应该期望得到它，因为您的代码是可疑的.

首先，如果仍然创建新任务，第一个片段中Parallel.ForEach的目的是什么?这个不成立.本质上，您说的是创建新任务不是顺序地，而是以并行和随机的顺序进行，因为创建的顺序无关紧要".但是创建的顺序无关紧要，仅因为它们是任务并且无论如何都并行执行，并且循环之外的代码几乎什么也不做.每次使用Parallel.ForEach时，您都应该想到可以通过在不同CPU内核中运行循环的某些不同迭代来提高总吞吐量.在这里你什么也得不到.相反，由于技术的开销，您失去了一些性能，而没有使用任何有用的效果.另外，您应该考虑迭代是如何独立的，这里不是这种情况.

现在，DoSomeWork可以执行多个任务.如果所有的身体都在锁下运转，那会产生什么感觉呢? (顺便说一句，避免使用lock(this).为什么?因为，您的类内部同步对于用户应该是透明的.特别是，类用户不应参与同步，在这种情况下，尤其是要锁定同一对象.由于实例用户始终可以访问"this"，因此不保留此安全规则.)因此，您正在做的是:首先创建并行任务以运行该方法，但是每个任务一次运行一次它的代码.您可以有效地完全消除上一步中获得的并行性.并行执行的唯一效果是:任务以随机顺序锁定.甚至这足以创建竞争条件.锁定的目的不同.您应该查明对共享数据的访问并锁定其使用.只有在锁下的操作非常快时，锁才是好的.最好的锁是无锁.数据结构应以多任务方式组织，以最大程度地减少锁定.

看看您做了什么:首先您过度使用了并行性，然后您完全杀死了并行性的任何有用效果(提高了吞吐量)，并且仅获得了它的负面影响，这就是竞争条件.

最后，为什么种族状况?我无法直接看到它，但可以看到它是如何被邀请的.查看FirstTask签名.此功能没有输出，只有输入.这仅意味着一件事:您不会通过堆栈返回任何内容，因此只能通过副作用来获取数据.您将数据放到一些共享内存中.您会看到，并行执行完全基于堆栈，每个任务都有自己的堆栈.您应该通过堆栈来隔离数据，而不是通过锁定和使用共享数据来隔离数据.因此，您需要进行计算，其中FirstTask的副作用会在随机的时间出现.难怪你有比赛条件.

您不仅击败了并行化的目的，而且还多次挫败了并行化的目的.您的代码作品在几个方向上都在与自己抗衡.在您的情况下，即使根本不使用多任务也会更好. (好吧，有一个线程用于数据库事务，如果有的话，一个线程就是UI；这很简单.这不是我的建议；我只是说不是您使用任务的方式比不使用任何方式要差得多.)

可以改进或修复您的代码吗?否.您应该从数据结构的设计开始重新考虑应用程序的设计.本地数据结构本身应考虑并行执行.您应该确定哪些数据部分在什么时间段内彼此独立，依此类推.

—SA

It''s hard to get into all details, but even from the first glance I can tell you''re trying to abuse technology. It looks like you''re trying to stick parallel execution by the principle the "more the better". Things does not work this way.

The negative effect you describe is called race condition. When you say variable get exchanged, you don''t explain which variable (and this is the problem, see below), but you should expect it, because you code is suspect.

First, what''s the purpose of Parallel.ForEach in first fragment, if you create new tasks anyway? It makes no sense. Essentially, you say "create new tasks not sequentially, but in parallel and in random order, because the order of creation does not matter". But the order of creation does not matter anyway, just because they are tasks and execute in parallel anyway, and the code in the loop beyond that does next to nothing. Every time you use Parallel.ForEach you should think of you can improve the total throughput by running some of the different iterations of the loop in different CPU cores. Here you cannot gain anything. Rather, you loose some performance due to overhead of technology without using any useful effect. Also, your should think how the iterations are independent, which is not the case here.

Now, DoSomeWork runs in several tasks. What sense it can possible make if all the body is run under the lock?! (By the way, as a rule of thumb, avoid lock(this). Why? Because, your class internal synchronization should be transparent for the user. In particular, there should not be a possibility for a class user to participate in synchronization, in particular, to lock on the same object. As "this" is apparently always accessible to the instance user, this safety rule is not held.) So, what are you doing is: you first create parallel task to run the method, but every tasks runs it code along, one at a time. You effectively completely kill the parallelism obtained in previous step. The only effect of parallel execution is this: the tasks come to the lock at random order. Even this is enough to create a race condition. The purpose of lock is different. You should pin-point the access to shared data and lock its use. The lock is only good when the operation under the lock is very fast. The best lock is no lock; the data structure should be organized in multitasking matter instead to minimize locking.

Look what have you done: you over-used parallelism at first, then you completely killed any useful effect of parallelism (improving throughput) and managed to get only the negative effect of it, which is race condition.

Finally, why race condition? I cannot see it directly, but I can see how it is invited. Look at the FirstTask signature. There is no output from this function, only input. It means only one thing: you do not return anything through the stack, so you get the data only through side effect. You put data to some shared memory. You see, parallel execution is totally based on stack, the fact that each task has its own stack. You should isolate data by stacks, not by locking and using shared data. So, your have calculation where the side effect FirstTask comes at random moment of time. No wonder you have race condition.

Not only you defeat the purpose of parallelism, you defeat it several times. You code works struggles against itself in several directions. In your case, even not using multitasking at all would be much better. (Well, one thread for database transactions, one thread is UI if you have it; quite simple. Not that this is my advice; I only say not your manner of using tasks is much worse than not using any.)

Can your code be improved or fixed? No. You should rethink the design of application starting from the design of your data structure. The local data structure itself should be designed in parallel execution in mind. You should identify, which parts of data are independent from others during what period of time, and so on.

—SA

这篇关于在并行foreach循环执行期间，变量的值会互换的文章就介绍到这了，希望我们推荐的答案对大家有所帮助，也希望大家多多支持IT屋！