CViewSelect::PrintSelectionInfoMsg の高速化、及び行のEOL長を取得する処理の高速化

[beru]

PR の目的

選択範囲情報メッセージの表示処理を速くする事でレスポンスを上げるのが目的です。

ステータスバーを表示している場合は選択範囲の文字数がそこに表示されます。

CViewSelect::PrintSelectionInfoMsg メソッドは選択範囲情報メッセージを作成してステータスバーに表示する処理ですが、そこの記述を更新しました。主に最適化したのは選択文字数をカウントする処理です。

カテゴリ

• 速度向上

PR の背景

ステータスバーを表示している場合の選択範囲操作のレスポンスが上がります。

2^24 (16777216) 行あるファイルを開いた後に、Ctrl + A を押して全選択した後に Shift キーを押しながらカーソルキーを押して選択範囲を変更すると結構操作のレスポンスが良くない事が確認出来ます。

実はこのPRの変更よりも #985 を適用した後の方が効果が高いです。それはレイアウト情報の確保をメモリプールを使って行っているので（メモリの利用効率が良くなった為に）キャッシュヒット率が向上している為と考えられます。

Mery というテキストエディタがありますが、このテキストエディタは様々な操作に掛かる時間がサクラエディタより大分高速です。

最近の一連のPRでパフォーマンスの改善を行う事で速度差を縮める事が出来ればと考えています。

このPRでの細かい変更内容について記載します。

従来実装では CLayoutMgr::GetLineStr メソッドが使われていましたが、CLayoutMgr::SearchLineByLayoutY メソッドで置き換えました。
取得した値のうちレイアウトデータしか使っていなかったので、CLayoutMgr::GetLineStr メソッドは使うまでも無いと判断したからです。

CEol::GetLen メソッドもアプリケーションの色々なところから呼び出されているのでインライン化されるようにヘッダ側に記述を移しました。
行終端子の長さのテーブルもヘッダファイル CEol.h 側に用意しました。CEol.cpp ファイルにある g_aEolTable 配列と一部内容が重複しているので保守の手間が増えますが処理時間を削減するには必要な事です。

PR のメリット

ステータスバーを表示しているケースで、行数が多い文書を扱っている際の範囲選択操作のレスポンスを改善します。

PR のデメリット (トレードオフとかあれば)

CEol::GetLen メソッドをインライン化する為にヘッダーファイルにテーブルを新規追加したので保守の手間が増えます。

PR の影響範囲

CViewSelect::PrintSelectionInfoMsg メソッド関連

CEol::GetLen メソッド関連

関連チケット

#985

[AppVeyorBot]

✅ Build sakura 1.0.2134 completed (commit be5be12080 by @beru)

[berryzplus]

改善の方向性としてGOな判断です。

CEol::GetLen メソッドもアプリケーションの色々なところから呼び出されているのでインライン化されるようにヘッダ側に記述を移しました。

サクラエディタのRelease版ビルド設定は、CMakeでいうRelWithDebInfo相当なので、ヘッダに移しただけだと（＝inlineを付けないと）効果がないはずです。

行終端子の長さのテーブルもヘッダファイル CEol.h 側に用意しました。CEol.cpp ファイルにある g_aEolTable 配列と一部内容が重複しているので保守の手間が増えますが処理時間を削減するには必要な事です。

似た配列を複製するのではなく、元配列をヘッダ側で参照できるようににしたいです。
※ extern struct XXX g_aEolTable[8]; にしたらいいんじゃないかと言ってます。型、数字テキトー。

2^24 (16777216) 行あるファイルを開いた後に、Ctrl + A を押して全選択した後に Shift キーを押しながらカーソルキーを押して選択範囲を変更すると結構操作のレスポンスが良くない事が確認出来ます。

事象確認のために使うファイルの行数が、常用外レベルに大きくなってる点についてはスルーします。

従来実装では CLayoutMgr::GetLineStr メソッドが使われていましたが、CLayoutMgr::SearchLineByLayoutY メソッドで置き換えました。
取得した値のうちレイアウトデータしか使っていなかったので、CLayoutMgr::GetLineStr メソッドは使うまでも無いと判断したからです。

妥当な判断だと思います。

ちなみに、これらの関数の引数型 CLayoutInt というのは int ベースの独自型です。
一年くらい前にissue登場済みの、やや問題ありな型です。
#326 マクロ関数の選択開始桁と選択終了桁で取得できる数値が桁数でない

Mery というテキストエディタがありますが、このテキストエディタは様々な操作に掛かる時間がサクラエディタより大分高速です。

ぼくの中では サクラエディタ > VS Code > Visual Studio な感じです。
最近は Java 触らなくなったので他のエディタはほぼ使わなくて済んでいます。
Meryへのライバル意識の発露のさせ方としては「絵文字対応」なんてのもあります。

っていうか、MinGWビルド直さないといかんですね・・・。

[beru]

サクラエディタのRelease版ビルド設定は、CMakeでいうRelWithDebInfo相当なので、ヘッダに移しただけだと（＝inlineを付けないと）効果がないはずです。

x64 Release ビルドで確認しましたが付けないでもインライン化されています。
クラス定義内のメソッドはデフォルトでインライン化されるからみたいです。

https://stackoverflow.com/a/13687207/4699324

似た配列を複製するのではなく、元配列をヘッダ側で参照できるようににしたいです。
※ extern struct XXX g_aEolTable[8]; にしたらいいんじゃないかと言ってます。型、数字テキトー。

その場合は SEolDefinitionForUniFile 構造体の定義をヘッダ側に移す必要があると思います。

事象確認のために使うファイルの行数が、常用外レベルに大きくなってる点についてはスルーします。

確かに極端な例なんですよね。隠れている問題を表面化するのには役立ちますがもうちょっと現実的なユースケースでの改善を試みないといかんですね。。

ちなみに、これらの関数の引数型 CLayoutInt というのは int ベースの独自型です。
一年くらい前にissue登場済みの、やや問題ありな型です。
#326 マクロ関数の選択開始桁と選択終了桁で取得できる数値が桁数でない

リリースビルドでは、#define USE_STRICT_INT されないので通常の int になるので問題ないのではないでしょうか？（訳：問題無いアル）

#326 は久しぶりに見ました。懐かしいです。

ぼくの中では サクラエディタ > VS Code > Visual Studio な感じです。
最近は Java 触らなくなったので他のエディタはほぼ使わなくて済んでいます。
Meryへのライバル意識の発露のさせ方としては「絵文字対応」なんてのもあります。

サクラエディタは色々なんとかしたい点がありますが「絵文字対応」は個人的には優先度が低いところです。サロゲートペア文字の扱いとか大変そうだし、ブラウザで見ればいいし…。

[berryzplus]

サクラエディタのRelease版ビルド設定は、CMakeでいうRelWithDebInfo相当なので、ヘッダに移しただけだと（＝inlineを付けないと）効果がないはずです。

x64 Release ビルドで確認しましたが付けないでもインライン化されています。
クラス定義内のメソッドはデフォルトでインライン化されるからみたいです。
https://stackoverflow.com/a/13687207/4699324

C++11(=言語仕様)で既定されてるんですね。
マニュアルも更新されてる感じです(/Ob1)。
（ms docsに誤植見つけたので指摘してきました 😄）

似た配列を複製するのではなく、元配列をヘッダ側で参照できるようににしたいです。
※ extern struct XXX g_aEolTable[8]; にしたらいいんじゃないかと言ってます。型、数字テキトー。

その場合は SEolDefinitionForUniFile 構造体の定義をヘッダ側に移す必要があると思います。

ブロックごとヘッダ側にコピペで何か問題あるんでしたっけ？
ってあれ？ForUniFileのほうでしたっけ？
g_aEolLengths に設定された定数を見た感じ、Uniなし版がベースのような。

ちなみにこんな感じでやってみたらビルドは通りました。

ヘッダ側
struct SEolDefinition{
	const TCHAR*	m_szName;
	const WCHAR*	m_szDataW;
	const ACHAR*	m_szDataA;
	int				m_nLen;

	bool StartsWith(const WCHAR* pData, int nLen) const{ return m_nLen<=nLen && 0==auto_memcmp(pData,m_szDataW,m_nLen); }
	bool StartsWith(const ACHAR* pData, int nLen) const{ return m_nLen<=nLen && m_szDataA[0] != '\0' && 0==auto_memcmp(pData,m_szDataA,m_nLen); }
};
extern const SEolDefinition g_aEolTable[];

実装ファイル側
const SEolDefinition g_aEolTable[] = {
// ↑先頭のstatic外しただけ

やや問題ありな型です。

レイアウト単位というコトバを、行を数える単位、桁を数える単位、画面上のピクセルを数える単位の3種類の意味に使っていて、何気にとり違いが起きてるっていう問題でした・・・。そのうちなんとかしたいですが、直接的には放置でＯＫです 😭

サクラエディタのなんとかしたい点
実は Shift_JIS の特定カテゴリの文字を表示できない、って不具合があったりします。

[beru]

その場合は SEolDefinitionForUniFile 構造体の定義をヘッダ側に移す必要があると思います。

ブロックごとヘッダ側にコピペで何か問題あるんでしたっけ？
ってあれ？ForUniFileのほうでしたっけ？
g_aEolLengths に設定された定数を見た感じ、Uniなし版がベースのような。

あ、SEolDefinitionForUniFile ではなくて SEolDefinition の方でした。構造体定義もヘッダに移しても多分問題は起きないと思います。

ちなみにこんな感じでやってみたらビルドは通りました。

お、確認ありがとうございます。

レイアウト単位というコトバを、行を数える単位、桁を数える単位、画面上のピクセルを数える単位の3種類の意味に使っていて、何気にとり違いが起きてるっていう問題でした・・・。そのうちなんとかしたいですが、直接的には放置でＯＫです 😭

過去に範囲選択領域の変更がピクセル単位になってしまって困ってましたが #738 で過去パッチを取り込んで解消したのを思い出しました。

今どういう問題が起きているか把握していませんが、知らぬが仏、という事でしょうか？

サクラエディタのなんとかしたい点
実は Shift_JIS の特定カテゴリの文字を表示できない、って不具合があったりします。

JIS第1水準以外の文字は憲法で禁止してほしいです。

[AppVeyorBot]

✅ Build sakura 1.0.2136 completed (commit ecba3164b9 by @beru)

[berryzplus]

/azp run

[berryzplus]

↑rebaseしないと意味無いんだった...orz

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[berryzplus]

レイアウト単位というコトバを、行を数える単位、桁を数える単位、画面上のピクセルを数える単位の3種類の意味に使っていて、何気にとり違いが起きてるっていう問題でした・・・。そのうちなんとかしたいですが、直接的には放置でＯＫです 😭

過去に範囲選択領域の変更がピクセル単位になってしまって困ってましたが #738 で過去パッチを取り込んで解消したのを思い出しました。

今どういう問題が起きているか把握していませんが、知らぬが仏、という事でしょうか？

桁位置取得のマクロ関数の挙動を、まだ直せてなかった気がします。
他には、アウトライン解析の内部処理で行桁の取り違いが発生していて、おそらく潜在バグなんだけど「報告もあがってないので放置」という状態が続いております:smile:

サクラエディタのなんとかしたい点
実は Shift_JIS の特定カテゴリの文字を表示できない、って不具合があったりします。

JIS第1水準以外の文字は憲法で禁止してほしいです。

ロボット憲法第４条・・・

今週中には PR できそうなんで書いてしまいました。
サクラエディタでは元ドキュメントを破壊しないために、SJIS⇒UNICODE変換ができてもUNICODE⇒SJIS変換で元に戻らない文字をバイナリ扱いにしています。この処理でバイナリ扱いになった文字（本来はUnicode変換できる）を文字として表示する機構がないので、表示上は文字化けしたように見えてしまうわけです。PR出すときに詳細出しますが、いわゆるOEM拡張文字の一部が表示できない状態になっとるわけです。(IBM拡張かNEC拡張かどっちだったか忘れました）

ほぼ関係ない話でした・・・

[berryzplus]

というわけで、問題ないと思います。
対応ありがとうございます。

[beru]

@berryzplus さん

レビューありがとうございました。問題が見つかったら別のPRで対処します。