おことわり
この記事はZennに書いていたシリーズを移したもので、初出は2023年11月19日である。
目次
今回の標的
クソコード仕置き人というからには標的はクソコードである。今回のクソコードは下記の記事より。
素晴らしい題とは裏腹に、令和4年に公開され令和5年に更新されているとは信じがたい嘘や不適切な処置が多数並んでいる。何度かに分けて丁寧にお仕置きしていこう。
クソコード生産者表示
クソコード仕置き人というからには標的はあくまでクソコードであるのだが、その量産業者がいるとすれば当然そちらも警戒せねばなるまい。
今回の生産者は堺康行…ClosedXML関連の情報を探していると度々検索上位に来る印象だ。個人企業の代表者らしい。私は純開発者なので、経理事務営業設計実装と一人でこなされていることは敬服するが、記事を読む限り少なくとも実装はなるべく早く誰かに任せるべきだろう。実際にプログラマを募集されているようだが、レビューと称してクソコードを強要される可能性から優れた技術者ほど応募を躊躇しそうではある。
クソコード概要
まずは生産者の記事から引用しよう。
次のコードのように、Whereで絞込した後にFirstメソッドで取り出すといった処理を行う場合、事前に即時評価を行った方が高速になります。
ケース①ではFirst実行時にWhereが未評価のため全件検査になる一方、ケース②では絞り込みが評価された後ですので、検索範囲が絞られた効果を得ることが出来ます。//事前準備。list変数は要素数1万のList<TestClass>クラスインスタンス //各要素のProp1にインデックスと同じ自然数を格納。(兼ウォーミングアップ) var list = new System.Collections.Generic.List<TestClass>(); for (int i = 0; i < 10000; i++) { list.Add(new TestClass() { Prop1 = i }); } //テストケース①即時評価せずFirstで取り出し var narrowedEnumerable = list.Where(x => x.Prop1 > 4000); var result = narrowedEnumerable.First(x => x.Prop1 == 4345); //テストケース②即時評価してからFirstで取り出し var narrowedList = list.Where(x => x.Prop1 > 4000).ToList(); var result = narrowedList.First(x => x.Prop1 == 4345);
瞬時に「そんなはずねえけどなあ」と思われた方には、続きが全く必要ない。以下には本当に当たり前の解説と検証しかない。しかし初心者がやたらとToArray()
やToList()
をしたがって困っている開発現場も多かろうから、しっかりお仕置きしておく。
お仕置き
早いものでLINQも生まれて15年以上になるが、基本はずっと変わっていない。下記にしたがえば間違えない。
- 可能な限り
IEnumerable<T>
のまま取り回せ - 2回以上結果を列挙する *1 場合、もしくは特定の時点での結果が必要な場合のみ
ToArray()
やToList()
などで確定させろ
したがって先ほど引用したコードにおいて採用すべきは「テストケース①」である。後に私からの説明も置いてはおくが、優れた記事はすでに多くあるので、それらから基本をしっかり把握すればまず問題ない。今回は次に挙げる記事を推薦する。
2021年の記事だが、この頃から急に遅延評価優先が流行するはずもなく、この記事もさらに3年半前に書かれた内容を刷新したものとある。いずれにしても、必要以上にToArray()
やToList()
したがるなということである。
「2回以上結果を列挙」ってなんや?の補足。推薦した記事で理解できれば読む必要はない。
「2回以上結果を列挙する場合は」とはいうが、そもそも何をしたら「1回結果を列挙した」と数えるのか?それは、あるIEnumerable<T>
に対して下記のような操作をすればいずれも「1回」分になると押さえておくとよい。
foreach
で要素を取り出し始める(仮に全要素を取り出さずに抜けたとしても)- 集計系のメソッドを呼び出す…
Sum()
,Count()
,Average()
, ...など - 探索系のメソッドを呼び出す…
First()
,ElementAt()
,Any()
,All()
, ...など
上記に含まれないSelect()
, Where()
, Take()
, Skip()
, Distinct()
*2などの呼び出しは「遅延評価」の性質を持つ。すなわち、呼び出しても即座に結果は列挙しないので「1回」分に数えない。必要になるまで結果の計算を遅らせることで、後から「有無だけ分かれば良かったわ」などと言われた場合に楽ができるよう構えているのである。あるメソッドが遅延評価なのか迷った場合はEnumerableクラスのリファレンス*3の該当するメソッドの説明を見に行くといいだろう。「注釈」欄にこのメソッドは、遅延実行を使用して実装されます。
*4とあれば遅延評価である。
裏取り
今回はクソコードがベンチマークなので計測法自体にも触れておく。素人がウォーミングアップ云々と計測法を気遣ったつもりでも限界がある。.NETのマイクロベンチマークでは事実上公式レギュレーションと言えるBenchmarkDotNetを用いるべきだ*5。仮にレギュレーション自体に問題があることが分かっても、オープンソースたるBenchmarkDotNetの修正を待って同一コードで再度計測することは容易である。これこそまさに関心の分離といえよう。
今回のクソコードについては、何故First()
一発で書ききらずにWhere()
を先に挟んでいるのかとも思ったが、「Prop1
が4000を超えることは業務共通の条件であるのに対し、その中から4345の要素を探すことは機能固有の要件なので分けて実行するのだ」とでも脳内補完することにした。TestClass
の詳細も分からないので、とりあえずint
型プロパティを増やしておいた。そうしてできた裏取り用コードと、その測定結果が下記である。
裏取り用コード
BenchmarkDotNet v0.13.10, Windows 10 (10.0.19045.3693/22H2/2022Update) AMD Ryzen 5 5600, 1 CPU, 12 logical and 6 physical cores .NET SDK 8.0.100 [Host] : .NET 8.0.0 (8.0.23.53103), X64 RyuJIT AVX2 DefaultJob : .NET 8.0.0 (8.0.23.53103), X64 RyuJIT AVX2
Method | Mean | Error | StdDev | Gen0 | Gen1 | Allocated |
---|---|---|---|---|---|---|
ToListCultist | 27.58 μs | 0.552 μs | 0.590 μs | 7.8430 | 1.9531 | 131472 B |
Normal | 10.74 μs | 0.200 μs | 0.385 μs | - | - | 72 B |
ToListCultist
が生産者の推奨する「即時評価してからFirstで取り出し」である。何の利点もないことがよく分かるだろう。実行時間が3倍弱ということに目が行きがちだが、1万件からの探索に対して10μs単位の差なのでここがボトルネックとして表面化する機会はあまりなさそうだ。それよりも注目したいのはAllocated
である。メモリを大幅に無駄遣いしていることが見て取れる。Gen0
, Gen1
が示すとおり不要なGCも促してしまっている。
戒め
LINQが遅いのではない。貴方の書いたコードが遅いのだ。
前回のHashSet<T>
のように、メモリを確保してパフォーマンス向上を図ることはあるが、メモリを浪費した挙句パフォーマンスを落とすいうのはクソコードそのものである。
再び生産者の記事から引用しよう。
(参考)ToList()の処理自体にはさほどの処理速度不可はかからない
次のようなコードで比較してみたところ、処理速度では大きな差は見られませんでした。(尚、メモリ効率の面では①の方が優れていると推測されます)// 引用者注:listの初期化部分は前掲と同じなので割愛する //テストケース① 評価は1回だけ var narrowedEnumerable1 = list.Where(x => x.Prop1 < 5000); var narrowedEnumerable2 = narrowedEnumerable1.Where(x => x.Prop1 < 2000); var narrowedEnumerable3 = narrowedEnumerable2.Where(x => x.Prop1 < 1000); var result = narrowedEnumerable3.ToList(); //テストケース② Whereするごとに評価する var narrowedList1 = list.Where(x => x.Prop1 < 5000).ToList(); var narrowedList2 = narrowedList1.Where(x => x.Prop1 < 2000).ToList(); var narrowedList3 = narrowedList2.Where(x => x.Prop1 < 1000).ToList();
「不可」は「負荷」のことだとして、計測法が不適切なうえに認識が甘すぎる。GCは全スレッド中断のうえで実行される重たい処理として有名で、それをメモリ浪費によって促してしまうのは避けるべきだ。万事がゼロアロケーションであるべきとは思わないが、資源は大切に使おう。
豪華特別付録 Count()ならどうなる!?
ほぼ同じ内容で回を分けるのもなんなので付録で済ませる。生産者の記事にはToList()
してからCount()
しよう(ToList()
後は当然Count
プロパティだが)という主張もある。そのコードを引用する。
//テストケース① 事前にToListせすCountメソッドを利用 var narrowedEnumerable = list.Where(x => x.Prop1 > 4000); if (narrowedEnumerable.Count() > 0) { foreach (var item in narrowedEnumerable) { var a = item.Prop1; } } //テストケース② 事前にToList var narrowedList = list.Where(x => x.Prop1 > 4000).ToList(); if (narrowedList.Count > 0) { foreach (var item in narrowedList) { var a = item.Prop1; } }
テストケース①を見ると、Count()
の呼び出しとforeach
による取り出しで「2回結果を列挙している」例なので結果を確定させてから操作すべきである…それ自体は間違いではないが、Where()
の条件に該当する要素が皆無の場合はforeach
を素通りしてくれるのでif
による要素数の判断がそもそも不要である。するとCount()
も不要で、それに伴いやはりToList()
も不要になってしまう。
「違うの!結果がないときは『ない』という特別メッセージを表示するの!!」とでも言うつもりなのかも知れないが、それなら最初からそうと分かる例を提示しないと無益だ。ループの中でローカル変数に各要素を上書きし続けるだけ(書いた値を利用しない)という処理も最適化で消されてしまいかねないので、ベンチマークには適さない。正しく計測できないと正しく高速化もできないだろう。
*1:クエリ内容が簡素で結果の要素も少ない場合、「IEnumerable<T>のまま2回連続でforeachさせても、ToArray()させたものよりわずかに速かった」という逆転もありえる。しかし、このわずかな差がボトルネックになることは稀だろう。
*2:個人的にDistinctが遅延評価は意外だった
*3:左の見出しで「列挙」と中途半端に自動翻訳されているのは勘弁してほしい
*4:「遅延評価」表記の方をよく見る気がするが、公称が「遅延実行」なのか、自動翻訳の妙なのかは分からない。
*5:ちょうど先日リリースされたVisual Studio 2022 17.8では新機能としてプロファイラとBenchmarkDotNetの連携が強化されており、今後も公式化が進むとみられる。