sbtにissue作るかpull reqを出したいけれど
「誰が見ても明らかなバグで修正方法も明らか」
ではなく*1、少し議論の余地がありそうだけれど、そのあたりを英語でうまく説明する自信がないので、まずは日本語で書きだしてみるシリーズ(たぶん後でissue報告かpull reqします しました) *2
- "~ compile" というのは、ファイルの変更を検知して、それをトリガーにして、コンパイル(やテストなどの指定した任意のコマンド)を実行してくれるもの。
- 「ファイル名変更や移動を検知してくれない」というの、確かにそうなのだが、別にそれほど困ってない
- ただ、以下のような構成になってるとき「rootで"~ compile"にしておいたとき、aからb(もしくはbからa)にファイルを移動しても、検知されない」というので、数ヶ月に一度くらいの頻度で困る
val a = project val b = project.dependsOn(a) val root = project.aggregate(a, b)
- 「このファイルはやっぱりこっちのモジュールにあったほうがいいなー」と整理する際や「1つだったモジュールを2つに分ける際」に、たまにモジュールまたいだファイル移動を行うことがあるので
- aからbや、bからaに移動だと、コンパイル結果全然異なるものになる場合がありえるので、できれば再コンパイルしてほしい
- 一度 "~ compile" の状態を解除して、再びコンパイルすると、正常にコンパイルされるので、まぁそれほど大きな問題ではない
- なぜこういう動作なのか？をソースコード読んで探ってみた
- 「ファイル数」と「ファイルの中の最終更新日時の一番新しもの」だけを保持して比較していたので、当然の結果*3
- https://github.com/sbt/sbt/blob/v0.13.5/util/io/src/main/scala/sbt/SourceModificationWatch.scala
- なので「監視対象のすべてのファイルのCanonicalPathを
Set[String]で保持して、毎回比較」と修正してみたのが以下 - https://github.com/xuwei-k/xsbt/commit/f1e50a9850
- とりあえず修正はしてみたが、それで悩んでるのが何点か
- 果たしてこういう動作にしてしまっていいのか？
- かなり使われてる重要な機能なので、パフォーマンス劣化したらやばそう
- Set[String]を保持して毎回比較するのは、速度やメモリ的にどうなのか？
- こういう動作にしてしまって、なにかデメリットないか？
- そもそも「プロジェクト間をまたいだファイル移動」だけを検知すればよい？(それは可能なのか？)
- 過去にこの件に関する議論やissueはなかった気がするけど、あったらすみません
- 動作を急に変更するのが怖いなら、この部分の動作を使う側がカスタマイズできるようにすべき？
- どういう方法で？
- Boolean型のKeyを1つ追加(今まで通りの動作か、自分が修正した動作か)
- Key追加ではなく -D などで指定する？
- Booleanではなく、たとえばWatchStrategyというようなものを作って汎用的にする(そんなことするほどのものか？)
- ところで、こういうテストどうやって書くべきなのだろう。scripted-testだと書けない？
ここから追記
以下、よこたさんとのやりとり
.@eed3si9n_ja ちょっとsbtのこと書いたので、できたらアドバイスとか意見をください URL
@xuwei_k 全体の issue 及び fix の方針は LGTM です。気になるのが、やはりパフォーマンスなので、現状から劣化するのか worst case と一般的なケースでデータがあればいいかな、特にソース濃い目のプロジェクトで。
2014-06-08 04:10:44 via YoruFukurou to @xuwei_k
@xuwei_k あと、Set[String] の比較と、ソートして List[String] 比較した場合と、ソートして mkString して Hash(...) を計算して文字列に変換して sha1 同士で比較した場合で有意な差があるかも気になる。
2014-06-08 04:14:06 via YoruFukurou to @xuwei_k
@eed3si9n_ja なるほど、ちょっと試してみます。動作切り替えられるようにはする必要はないですかね？(もし切り替え可能にするならどういう方法がいいか？) 取り込まれる余地あるなら、少し試してパフォーマンス測ってみた後に、たぶんpull reqします
@xuwei_k 僕個人の意見としては、動作がより「正しく」なり、かつ互換性に問題が無いならスイッチは必要無いと思います。pull req 送ってから @jsuereth が何と言うかにもよるけど。
2014-06-08 04:21:42 via YoruFukurou to @xuwei_k
というわけで、言われたとおりに、まず
- 自分が最初に実装したSet[String]
- ソートして List[String]
- ソートして mkString して Hash(...) を計算して文字列に変換して sha1 同士で比較
を試す。
↓
結果的に速度的に優位な差はでなかった(2割くらい差がでた場合もあったが、数割ではほとんど誤差の範囲？)
なので、とりあえず自分の最初のSet[String]での比較でいい気がした。
あと、「ScalaのコレクションよりJavaのコレクションのほうが速いことがある」というのを耳にした覚えがあったので、java.util.HashSetでも試したが、やはり変わらなかった。
あと、念のため、scalaのimmutableなHashSetの実装見てみたが、
https://github.com/scala/scala/blob/v2.10.4/src/library/scala/collection/GenSetLike.scala#L114-L123
まず最初にsizeを比較してたり、その後もべつにアルゴリズムのオーダー的にひどくなるようなことはしていなさそうだった。
実際の
「getCanonicalPathして、Set[String]生成して、それを以前のSet[String]と比較」
の速度だが、Scalaz、play2、sbt、で試した結果、自分の手元のPCでは遅くても20msから30ms程度だった。たぶんこれなら許容範囲のはず(?)
また、しばらく "~ compile" のままにしておいても「使用メモリがモリモリ増え続ける」とか「CPU使用率が以前より明らかに大幅に増加する」とかはなかったと思う。CPU使用率は少しだけ増えるかも？(これも、対して慎重に計測したわけではないので、誤差なのかどうなのか不明)
というわけで、最初のコードのままpull reqした
