VisualStudioのバージョンによるOrderByの動作が違う

Question

いつもお世話になっております。
昨日家でコードを書いていて遭遇した問題と疑問点を質問させていただきたく思います。

前提

自宅ではVisual Studio 2019 Community Edition
Visual Studio 2017 Community Edition Version 15.5.7
仕事場ではVisual Studio 2013 Professional Edition
を利用しています。
それぞれ 2017の環境、2013の環境と呼称することとします。

該当のソースコード

コンソールプロジェクトを作成して以下のコードを実行します。
0～99の値をシャッフルして1行ごとに表示するといった、とてもシンプルなコードです。

using System;
using System.Linq;

namespace ConsoleApplication1
{
    class Program
    {
        static void Main(string[] args)
        {
            var order = Enumerable.Range(0, 100).OrderBy(x => Guid.NewGuid());
            Console.WriteLine(string.Join("\r\n", order));
        }
    }
}

発生した現象

2013の環境で実行した結果

4
7
11
9
8
...以下略

100行表示され、各行の値が重複なくシャッフルされた状態で表示されました。

続いて
2017の環境で実行した結果

97
1
0
1
27
...以下略

100行表示されますが、各行の値が一部重複し、欠番が発生しています。

2013の結果が偶然の可能性もあったため、100回程試しましたが、重複はありませんでした。
また、重複する値や個数は決まっているわけではなく毎回異なります。

問題を回避した方法

試しに以下のコードに変更して実験してみました。

// var order = Enumerable.Range(0, 100).OrderBy(x => Guid.NewGuid());
var order = Enumerable.Range(0, 100).ToList().OrderBy(x => Guid.NewGuid());

上記に変更した結果として、2013でも2017でも実行結果は
100行表示され、各行の値が重複なくシャッフルされた状態で表示されました。

疑問点

2013～2017の間で上記のようにIEnumerableに対してOrderByを書いた場合の動作が変わったのでしょうか？
それとも、自分の環境でしか起こらない現象なのでしょうか？

フレームワークのバージョン

2017 → .Net Core 2.0
2013 → .NetFrameWork 4.5

デバッグモードでの表示

追記依頼の部分でtamoto様からお聞きした情報からもう一度デバッグを行ってみました。

var order = Enumerable.Range(0, 100).OrderBy(x => Guid.NewGuid());

こちらのorderの中身を覗いてみると以下のようになりました。

コメントに画像が挿入できないためこちらに記載させていただきました。

Comments

[Tokeiya3]

VisualStudio2019で実行した際の、実行環境は何になるのでしょうか?
また、同様にVisualStudio 2013で実行した際の環境は何でしょうか?

尚、蛇足ながら、数列のシャッフルにGUIDを使うのはかなり悪手だと思います。

[shiroman]

両者ともWindows 10 64bit環境です

[Tokeiya3]

自分の環境(Windows10 64bit Net core 3.1)で実行したところ、問題なく動きました。

但し、個人的にはかなり危うい書き方をしている気がします。
(精査してないので、戯れ言程度に受けて下さい

[x_x]

その位置の .ToList() で正しく出ますかね？
手元の 2015 環境で試したところ、このように Guid 値を確定後でないと正しく出力されないようでした。
Enumerable.Range(0, 100).Select(a => new { Order = a, Guid = Guid.NewGuid() }).ToArray().OrderBy(a => a.Guid)

[Tokeiya3]

軽く調べたんですが、Guid構造体自体、同値比較は可能ですが、大小比較そもそも持ってないです。
それでOrderByした場合、Comparer<Guid>.Default.Comparerが呼ばれるので、もしかしたらその辺が問題になるのかも知れないです。

[YAmaGNZ]

Frameworkのバージョンは何なんですか？
こちらで.Net Framework3.5と4.7.2で比べてみましたが、どちらも正常動作しているようです。

[YAmaGNZ]

Tokeiya3さん横からすみません。Guid構造体には
https://referencesource.microsoft.com/#mscorlib/system/guid.cs,171e09ed65ddf443,references
こちらにあるようにCompareToが実装されていてこれがOrderByに使用されると思っていたのですが間違っていますでしょうか？

[退会済みユーザー]

Guid を生成しながらそれで OrderBy というところが引っかかります。それで期待通りになるという保証はなさそうな気がします（うまく行ったのはたまたまかも）。x_x さんがコメントに書かれたやり方が正解のように思います。

[hihijiji]

VS2019 .NET Core 3.0 でも重複や欠番は確認できませんね。
いったいどのような環境で実行しているのでしょうか？

[Tokeiya3]

>YAmaGNZさん

ご指摘多謝です。
Operatorしかみてなくって<>が無かったので、早とちりしてました。
Guid構造体はIComparable<Guid>持ってるので、比較可能でした。

[shiroman]

みなさん、ありがとうございます。
0～99でランダムに10個数字出すアプリを作って欲しい知り合いに頼まれてササッと書いたときに見つけたものだったので、実装方法に関してツッコミどころはあるかと思います。

重複なしのシャッフルされた数字リストを作るやり方としてOrderByとGUIDを用いたやり方は一般的によくやる手法だと思っているのですが、、、、違うのでしょうか。
数列のシャッフルにGUIDを使うのはかなり悪手というのが、すいませんが、私には理解できてないです。

.NetFrameworkのバージョンに関しては、恐らくデフォルトですが、家でソリューション開いてみないとわからないので追って書かせていただきます。

[Zuishin]

本当にこのコードで再現するのか疑問です。大小比較がうまくいかない場合、ソートはできないかもしれないけど重複や欠番が発生するとは思えません。この後に何かコードがある気がします。

[hihijiji]

OrderByとGUIDは私もよくやりますし、それで問題が起きたことはありません。
LINQと遅延評価の扱いのミスは私もよくやらかしますし、それでたびたび問題が起きます。

[Zuishin]

10 個取り出すというところで思いついたことがありますが、シャッフル後の数列を保存していないために 1 つ取り出すたびに評価されているのではないかと思います。

[Zuishin]

hihijiji さんが既に書かれていました。

[shiroman]

Zuishin様
今なんかギクッとなりました。
よく考えてみれば回答に書いていただいたコードにほぼ近いことやっていた覚えがあります。
デバッグした時に
var order = Enumerable.Range(0, 100).OrderBy(x => Guid.NewGuid());
の次の行でorderをスコープしたとき、既に同じ値が複数発生していたため、この行で既に問題が起こっていると勘違いしていたかもしれません。
（※デバッグでスコープすると評価前で値がまだ不定だから？その時点で重複してるのも変なような…）

本来は実装時点のコードをそのまま貼り付けるべきだったのですが、私が勝手に勘違いして問題であろう部分だけを書いたことで皆様に混乱をさせているかもしれません。
改めて家に帰って質問と同じソースコードで実験してみます。
誠に申し訳ありません。

[tamoto]

OrderBy + Guid は強固なランダム列を得たい場合によく使われる定型文ですので、シャッフルに利用することは全く問題ないですよ。

デバッグという発言でちょっと気になったのですが、デバッガ上の Result View は、ステップを進めて開く瞬間に独自に再評価されるため、画面上への出力とデバッガ上の値が一致することはまずなく、また開く度に結果が変動します。
そのあたりの、別の評価結果同士を見比べてしまった勘違いではないでしょうか。

[shiroman]

tamoto様
質問文にその時の状態のSSを添付しました。やはりデバッグモードでスコープしたときに値がすでに重複してしまっています。

[shiroman]

x_x様
>その位置の .ToList() で正しく出ますかね？
おっしゃるとおり、場所が間違えておりました。
var orders = Enumerable.Range(0, 100).OrderBy(x => Guid.NewGuid()).ToList();
が正解でした。

[shiroman]

皆様のおかげで問題点の解決に至りました。
最終的にはチョンボだったということでお恥ずかしい限りですが、本当に感謝しております。

しかし、問題は解決したのですが疑問は残っています。
質問文の「デバッグモードでの表示」節で追記したのですが、2017(.NetCore2.0)だと、デバッグモードでスコープすると頻繁に重複が起こり、でもこの後ToListしてから出力すると重複が起こらない
しかし、2013(.NetFramework4.5)ではデバッグモードでも重複は起こらないというのがやはり腑に落ちないです。
問題の性質上、チェックしたのはそれぞれの環境で20回程度なのですが、2013が重複しないのはたまたまなのでしょうか…。

といっても、これは別件に近いので一旦BAを選ばせていただきました。
本当にありがとうございます。

[tamoto]

画像確認しましたが、これが出力されたというのは非常に奇妙です。少なくとも現行バージョンで起きるのであれば再現プロジェクトと共にバグ報告をするべき事象と判断します。
netcoreapp2.0 ということなので、だいぶ古い環境のためこちらで試すこともできずでしたが、LTS のバージョン (netcoreapp2.1 / netcoreapp3.1) で試したときにも同じ症状が出るかどうかが気になります。

[shiroman]

ほんと、そのとおりですね。。。
一度フレームワーク変えて試してみます。
また、2019でも同様に起こるかを確認してみます。

[Zuishin]

表示されてるのは randkeys じゃないんでしょうか。

[Zuishin]

と思ったけど違いますね。スクロール時に再評価されてそうな気がします。

[hihijiji]

デバッグモードでの表示　の問題は、何かしらの拡張機能が割込みを入れている予感がします。
一度拡張機能を全て無効にしてから再起動して試してみてください。

[shiroman]

ResultView展開時に1要素ごとに再評価が走るような動きになっているとしたらこのような結果になりそうですが、それだとしても、環境かC#のバージョンか、何かが原因でデバッグ時の動きが違うっていうのも変な話ですよね。。。
バージョン変えたりして試してみます。

Answer 1

ご提示のコードでは string.Join("\r\n", order) の所で一度しか評価しないはずですので、重複は発生しないはずです。
重複が発生するなら何か勘違いしているか実行環境がバグってます。

参考までに重複が発生するコードを書いておきます。

var order = Enumerable.Range(0, 100).OrderBy(x => Guid.NewGuid());
for (int i = 0; i < 100; i++)
    Console.WriteLine(order.Skip(i).First());

これはループの中で再評価しているので重複が発生します。

Comments

[shiroman]

hihijiji様

上のやり取りでも少し書かせていただいたのですが、たしかに、同じようなことをやっていました。
おっしゃるとおり私の勘違いな気がします。。。

私が書いた　string.Join("\r\n", order)だと、一度の評価で文字列化されていますが
ご提示頂いたやりかただと、ループ回数分評価が発生して、毎回0...99の数列が生成されてシャッフルされて先頭が取り出されて…といった動作になるということですよね

なんか後からコロコロ仕様が変わった後にそういう書き方をしたような記憶が…。

[hihijiji]

遅延評価のIEnumerable<T> を表示用コントロールにバインドしていて、表示/再表示のときに再評価されるってのはありがちですね。

[shiroman]

やはりfor文内で毎回評価される書き方をしておりました。。。
まったくもってお恥ずかしい限りです。
鋭い回答ありがとうございました。

Answer 2

別のアプローチで生成すると、このようになります。もし興味があれば。
一番下の、TestWithArray()　メソッドから見てください。

public class RangeRandom
{
    public static IEnumerable<int> Next(int from, int to)
    {
        var rand = new Random((int)(DateTime.Now.ToFileTime() & 0x0ffffff));
        var array = Enumerable.Range(from, to).ToArray();
        for (var i = array.Length - 1; i >= 0; i--)
        {
            var index = rand.Next(0, i);
            var current = array[index];
            array[index] = array[i];
            yield return array[i] = current;
        }
    }

    public static int[] All(int from, int to)
    {
        var rand = new Random((int)(DateTime.Now.ToFileTime() & 0x0ffffff));
        var array = Enumerable.Range(from, to).ToArray();
        for (var i = array.Length - 1; i >= 0; i--)
        {
            var index = rand.Next(0, i);
            var current = array[index];
            array[index] = array[i];
            array[i] = current;
        }

        return array;
    }

    public static void TestWithEnumerator()
    {
        var check = 0;
        foreach (var item in RangeRandom.Next(1, 100))
        {
            Console.Write(item + " ");
            check += item;
        }

        Console.WriteLine();
        Console.WriteLine(check);
    }

    public static void TestWithArray()
    {
        var array = RangeRandom.All(1, 100);
        Console.WriteLine(string.Join(" ", array));
        Console.WriteLine(array.Sum());
    }
}

Comments

[shiroman]

なるほど、どちらもToArrayで一度配列にしてRandomクラスでインデックスを取り、値同士を入れ替えてるということですね。
Allの場合は完成品を返して、NextはIEnumerableで必要なタイミングで処理を動かすが、一度Arrayにしているから入れ物自体の再計算は走らないということですね。
guidの使用はパフォーマンス的にどうなのかな？と以前から疑問に思っていた部分もあるので、こういったクラスをライブラリとして用意しておいて使うってのも良さげですね。

参考になりました、ありがとうございます。