suconbu commented 6 months ago

PR対象

アプリ(サクラエディタ本体)

カテゴリ

改善

PR の背景

概要

サイズの大きいファイルを開く時のもたつきが気になるため改善を行います。調査には VisualStudio 2022 に付属するパフォーマンスプロファイラーのフレームグラフ機能を使ってみました。調べた結果を元に、本PRでは以下の2点の改善をします。

IsVariationSelector 呼び出しに伴う string_view コンストラクタの実行を回避
IsVariationSelector からの ConvertToUtf32 の呼び出しを削除

詳細

以下はファイル読み込み時のフレームグラフです。今回は比較的割合の大きい「1」「2」の箇所に注目しました。

※Debugビルドにて調査、調査時は USE_STRICT_INT の定義を無効化

1 について

CNativeW::GetSizeOfChar から IsVariationSelector を呼び出す際、std::wstring_view の (3) ※のコンストラクタが呼び出されています。 ※https://cpprefjp.github.io/reference/string_view/basic_string_view/op_constructor.html

constexpr basic_string_view(const CharT* str);                 // (3)

このコンストラクタでは、入力されたポインタを起点に文字列末尾までの長さをカウントを内部的に行うようで、呼び出し回数が多い場合にそれが負荷になっているようです。 (例：1億文字を含むファイル読み込んだ場合、IsVariationSelector の呼び出し回数は 197,721,600 回)

対策としては IsVariationSelector の呼び出し時に、計算量が定数時間となる (5) のコンストラクタを明示的に使うようにします。

constexpr basic_string_view(const CharT* str, size_type len);  // (5)

また、CNativeW::GetSizeOfChar と同様に比較的長い文字列を IsVariationSelector に渡す可能性のある CWordParse::WhatKindOfChar, CTextMetrics::GenerateDxArray についても同様の修正を行うことにします。

2 について

現在の IsVariationSelector の実装では異体字セレクタ判定のために一時的に UTF-32 へ変換を行っていますが、呼び出し回数が多い場合にはここが負荷となっているようです。

対策としては UTF-16 のまま直接異体字セレクタの判定するようにして UTF-32 への変換を省略します。

効果

-	CDocFileOperation::FileLoad	うち CLoadAgent::OnLoad	うち CLoadAgent::OnAfterLoad
master (47d5dc1e3)	6487	4025	2425
改善後 (8df88f1)	3422 (-47%)	2444 (-39%)	952 (-60%)

※単位：ミリ秒 ※計測に使用したファイル：100m_chars_utf8.zip を展開 ※Releaseビルドにて計測、時間計測には CRunningTimer を使用、3回計測した結果の平均

PR適用後のファイル読み込み時のフレームグラフです。IsVariationSelector が占める割合が小さくなりました。

仕様・動作説明

ユーザーに対する機能的なふるまいの変化はありません。

PR の影響範囲

特にありません。

テスト内容

計測用ファイル (100m_chars_utf8.zip を展開) を開いた時に読み込みが速くなっていることを確認します。
CIのテストで失敗が出ないことを確認します。

参考資料

AppVeyorBot commented 6 months ago

:white_check_mark: Build sakura 1.0.4343 completed (commit https://github.com/sakura-editor/sakura/commit/8eb83c2ab7 by @suconbu)

suconbu commented 6 months ago

問題に対して提示された対策の内容が飲み込めませんでした。定数時間になる (5) のコンストラクタを使用しないのはなぜでしょうか？

負荷の掛かっていた該当箇所 (CNativeW::GetSizeOfChar からの呼び出し) 以外にも、既存で IsVariationSelector を呼んでいるところも同様に不要な長さカウントがされないようにしようとしたのですが、そうした時に5か所ある既存の呼び出し箇所はすべて wchar_t* 型で渡すようになっていたことから、IsVariationSelector の引数をそれに合わせた、ということになります。 (そうすることで明示的なコンストラクタ呼び出しを都度書く必要もなくなることと、今後新たに IsVariationSelector を使おうとした人がうっかりコンストラクタを使わず呼んでしまい、不要な長さカウントが発生する心配もなくせるはず・・・)

AppVeyorBot commented 6 months ago

:white_check_mark: Build sakura 1.0.4344 completed (commit https://github.com/sakura-editor/sakura/commit/b4462a5a14 by @suconbu)

suconbu commented 6 months ago

頂いたコメントを参考に、IsVariationSelector の引数/呼び出しについては以下の内容で対応したいと思います。

IsVariationSelector の引数は (従来通り) std::wstring_view 型のみとする。
CNativeW::GetKetaOfChar から IsVariationSelector を呼び出す時に、std::wstring_view の (5) のコンストラクタを使う。(＝解消したい問題に対する修正)
比較的長い文字列を IsVariationSelector に渡す可能性のある CWordParse::WhatKindOfChar, CTextMetrics::GenerateDxArray にも同様の修正を行う。(＝ついでの修正)

上記反映後で計測すると当初の修正方法よりも少しだけ処理時間が増加していますが、大勢には影響なさそうです。