Dictionaryへのアクセスがデッドロックになる

Question

お世話になっております。

非同期処理でList<Dictionary<ParamType,double>>にParamTypeごとの計算処理を行いたいと思っています。

           var adjust_params = Enumerable.Range(0, node_depth).Select(index =>
                     new Dictionary<SamplingKey, double>() {
                        {ParamType.a,0},
                        {ParamType.b,0},
                        {ParamType.c,0},
                  
           ).ToArray();

            Parallel.ForEach(nodes.Values, node =>
            {
                int gen_counts = node.tables.AsParallel().Sum(data => data.generate)

                var ab_task = Task.Run(() =>
                {
                    double y1 = 0;
                    double y2 = 0;
                    for (int i = 1; i < gen_counts; i++)
                    {
                        double y = generateOrNot();
                        y1 += 1 - y;
                        y2 += y;
                    }
                    return new { a = y1, b = y2 };
                });

                double c = Math.Log(node.price);

                lock(adjust_params[node.depth]){
                    adjust_params[node.depth][ParamType.a] += ab_task.Result.a;      
                    adjust_params[node.depth][ParamType.b] += ab_task.Result.b;
                    adjust_params[node.depth][ParamType.b] += c;
                 }
            });

この処理ではnodeの数が多すぎるとlock(adjust_params[node.depth])でデッドロックになります。恐らく、二つ以上のスレッドが運悪くクリティカルセクションに侵入してしまうとadjust_paramの読み込みがロックされてデッドロック状態になってしまうようです。nodeがそこまで多くなければ問題なかったのですが、CPU使用率が100%になったりするので、遅延発生で同時にクリティカルセクションに侵入してしまうnodeがあるのかと思っています。やってることは重たい処理を並列化して、nodeごとの合計を出したいだけなのですが、どのように改善するのがよいでしょうか。

アドバイスいただける方いましたらよろしくお願いいたします。

Answer 1

この手の計算処理は、generateOrNotが相当時間かからない限りパラレルにしたりTask.Runした方がむしろ遅そうではありますが…

とりあえず以下のように書き直した場合ロックされますか？

Task.Run(() =>
{
    double y1 = 0;
    double y2 = 0;
    for (int i = 1; i < gen_counts; i++)
    {
        double y = generateOrNot();
        y1 += 1 - y;
        y2 += y;
    }
    return new { a = y1, b = y2 };
}).ContinueWith(x =>
{
    // ここでa, bを外に出す必要性は無いはずだが、こちらの方が分かりやすいし安全なので。
    double a = x.Result.a;
    double b = x.Result.b;
    double c = Math.Log(node.price);
    lock (adjust_params[node.depth])
    {
        adjust_params[node.depth][ParamType.a] += a;
        adjust_params[node.depth][ParamType.b] += b;
        adjust_params[node.depth][ParamType.b] += c;
    }
});

Comments

[退会済みユーザー]

こちらサンプルは実際のコードを書き直してかなり簡略化していますが、実際は内部でもっと重たいタスクが走っています。generateOrNodeはものすごく重たい処理と考えて差し支えないです。とりあえずやってみますね。

追記
一応計算回るようになりました！ありがとうございます！ただ、理屈があまり整理できていないのですが、同期処理になるのでContinueWithはThreadごとには排他制御になるとは思うのですが、それでもうまくいくのであれば並行スレッドじゃなくてメインスレッドに投げるからですかね？(同期スレッドで実行されるならlock)も消せる気がしました。ありがとうございます。

[haru666]

ふと思ったのですが、adjust_paramの値の取得処理が途中でなければDictionaryではなくクラスにして
class ParamValue
{
public double a;
public double b;
public double c;
}
とすれば、ロック不要でアトミックな操作が直接できたりしませんか？

[退会済みユーザー]

あぁ確かにそうですね...。なんか頭でっかちになってました。ありがとうございます。
haru666さんのコードがadjust_paramをロックしてたのは外での書き出し読み出しを考慮してるからですよね？
ありがとうございます。
追記　
ContinueWithは同じスレッド上でタスク後の処理を継続するようなので、ほかのスレッドからも書き込みがあるので、やっぱロックも入れないとだめなきがしました。上手くいってるのは、いったん同じスレッドで同期をすることで、ほかスレッド上でロック前の同期待ちが発生する確率が小さくなるからな気がしています。
また、思い出したのですが、dictionaryにしたのはadjust_paramsの要素は本来は構造体でロックの時にlistのインデックスごとにロックするのができなくてDictionaryにした経緯があったと思います。classにすれば同じようにロックできると思いますが、ロック省くのは無理そうなきがしました。