えるくりん月報　2024年3月号

3月号です。

今年度も終わりらしい。

今月も仕事やその他のことについてランダムに書きます。

Damage Calculationを見直してパフォーマンスを改善した

改善、というか半年くらい前に自分が導入したシステムの副作用で悪化したPerformance Regressionをようやく直した話。

フレームの一部を切り落とすための clipping という概念がある。フレームは必ずしもウィンドウの中に収まってるとは限らず、例えばスクロール中のYoutube動画はフルの画面がひとつのフレームだが上のほうがウィンドウの上になって隠れていることがある。この時、画像全体を生成するがその一部を形に合わせて"clip"している。

この clipping をサーバー側で一括して処理したいが、root surfaceの外側にあるものはサーバー側で扱えなかったので当時クライアント側で適宜clipしてから渡していた。これを直すために、サーバー側でroot surface外のフレームをサポートできるよう、外側のフレームが起点となるように座標変換をする変更を昔行った(Out-Of-Window clip delegation、例えばCL1, CL2, CL3 のあたりがメインの変更)。

パフォーマンス改善が見られた箇所があった一方で、特定のシナリオではこの変更がJank フレームを10倍くらい増やしていた。理由は、外側にある関係のないエリアが既存のロジックでうまくサポートされておらず、毎回full damage認定になっていたこと及び、rootとサイズが違うことでSurface IDの更新+opacityが更新されていたことなどにより過負荷がかかってdropped frameが増えていた。

なので、実際に影響する範囲のみを考慮する変更を加えて修正した。→CL: Exo: Calculate output and damage rect only from effective area

なお、root surfaceの外側にフレームを表示したいこともあり、必ずしもroot surfaceの外側がclipされるわけではないので、ただ全部clipするだけでは駄目。

Window State と Insets を configure-sync しよう

2月号に引き続き取り組んでいる。

すでに2回landしてrevertされているが、ようやく全てを直す変更のreviewが終わったところ。

wayland: Support window state and insets in configure-sync model

Insetsを直すためにwindow stateのsyncを正しくするCLを出したら、Insetsを直したCLと同じ理由でrevertされた。両方同時に存在しないと既存のテストが壊れる（既存の実装がすでに壊れているが、それをキャッチするテストはなく偶然動いている）。同時にランドする必要があり、CLがでかくなった。
Insetsをsetする方法が自明ではなく4つくらいのプロトタイプを用意してどれがいいか考えた。InsetsはWindow stateが変わったことをトリガーに変わるが、Stateのリクエスト中にOnWindowStateChangedが呼ばれてそのまま新しいInsetsに対応するStateをリクエストするとre-entrancyが発生し、vizのidの順番がインクリメンタルじゃなくなってしまう可能性があるのでなんとかする必要があった。セットしたいinsetsを持ち回って全てが終わったあとにセットする方法や、各ObserverからInsetsの変更値を集積して返り値として返したものをその場でセットする方法や、セットのためのCalbackを持ち回ってそれを呼んでもらうか、いろいろ考えたのち、Insetsを事前計算して事前セットすることにした。どれにするかの判断材料として、「コードをよく分かっていない人が間違ったInsetsの処理をするコードをlandしてしまわないようなデザイン」と「複雑度をあげないデザイン」で悩んだ。特に複数プラットフォーム(wayalnd, X11, テスト用ダミープラットフォーム)全てに対して良いデザインをするのが困難だったが、保全性と簡易性のちょうどいいバランスのデザインになったと思う。

Gardener

またChromeOSのビルドを監視する週間 ChromeOS Gardener の担当が回ってきた。2ヶ月に1回回ってくるの負担が大きすぎる気がするぞい。今回も圧倒的生産性でたくさんの問題をなぎ倒した。すごい。

今回は初めてshadowがついたので少し指導したが、その後は最初からしっかり仕事こなしててすごかった。

その他直したバグなど

ブラウザがterminateした時にrelaunchリクエストを投げても特定のタイミングだとリクエストが不当に破棄されているというrace conditionのロジックを発見。直した。Lacros有史以来ずっと生きながらえていたバグっぽい
HitTestMaskの座標計算ミスを直したやつが再発した。と思ったが、再発ではなく昔のバージョンのOSでは壊れっぱなしなだけだった。しかし、壊れているOSでは機能をdisableしないといけなかったことを忘れていた。直した。こういうversion skew問題をちゃんと検出できるテストフレームワークを導入したい。考え中。
X11 platformでinsetsをセットする関数の中で勝手に値を上書きしていたので直した。が、window stateがminimizeのときを考え忘れていてrevertされた。ちゃんとtree host側でminimizeのinsetsを正しくセットするように修正した。
ポリシーの設定部分にバグがあることを発見した。とある機能をenableするためのpolicyが能動的にセットされていないっぽくて、他のpolicyの設定をする際に巻き込みでセットされていただけだったため、ほかの部分をいじったら壊れた。一旦直したけど、根本的な原因は要調査。
BrowserManager周りのtest coverageを増やしたり、Chromeへの依存を消したりなどの整理活動を引き続き行っている。Technical deptを許さない。
Flakyテストと出会ったらdisableするな！直せ！Part1。ShelfWebAppBrowserTestはRunUntilIdleで全てのタスクが終わるまで待つという方法でBlockしていたが、Postしていたタスクが終わってからそのコールバックが次のTaskを投げる間に一瞬のIdle時間が発生する。このときにRunUntilIdleが正しく待ってくれない。多くの場合は、Chromeに存在するいろいろなTaskが走りっぱなしでIdle状態にならないためFlakyになっていた。RunUntilIdleではなく真に待ちたいタスクへDoneコールバックを渡し呼んでもらうことで解決。
Flakyテストと出会ったらdisableするな！直せ！Part2。ProxyManagerTestはUnretainedのdanglingチェックによりflakyになっていた。どうやらmojoの中のどこかに、base::Unretainedで呼び先ポインタを受け渡しているコールバックが呼び終わってないらしいので、とりあえずTearDownでRunUntilIleを呼び出して直した。が、mojoの中のunretainedを探したほうがいいね。
上記のパフォーマンス向上の変更の際、副作用でCrashを引き起こしてしまった。シンプルにDCHECKをCHECKにしたらたまにCrashするようになったので戻した。
３本指のタッチイベントの処理がおかしいバグ。一回のタッチでは一種類の挙動をしてほしいが、overviewに入るのとtabを移動するのが同時に起きてしまうことが稀にあるらしい。普通はeventのpropagationは実際にeventを消費したときに止まるべきだがそうなっていなかった。Ash側はTab移動終了時のイベントを止めることで解決。一方でLacrosについては少しrefactoringが必要。tab移動のイベントはcrosapiを通じているのでAsh側が実際にLacrosがイベントを始めたか判断するすべが現状ない。なので、本当にイベントを消費するべきときのみIPCを送るようにロジックを組み替える必要がある。WIP.