ファイル読み込みの高速化 #1951

suconbu · 2024-05-11T10:32:56Z

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 (`47d5dc1`)	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のテストで失敗が出ないことを確認します。

参考資料

suconbu · 2024-05-11T10:36:35Z

tests/unittests/test-ctextmetrics.cpp

@@ -225,7 +225,7 @@ TEST(CTextMetrics, GenerateDxArray8)
 	// IVSのVariantSelectorが続く文字列は先頭1文字 + 幅0×2で生成する
 	std::vector<int> v;
 	FakeCache1 cache;
-	CTextMetrics::GenerateDxArray(&v, L"葛󠄀", 2, 0, 0, 0, 10, cache);
+	CTextMetrics::GenerateDxArray(&v, L"葛󠄀", 3, 0, 0, 0, 10, cache);


長さ指定が間違っていそうなのでついでに修正します。(PR反映後のテスト実行で失敗したため判明)

AppVeyorBot · 2024-05-11T11:28:55Z

✅ Build sakura 1.0.4343 completed (commit 8eb83c2ab7 by @suconbu)

kengoide

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

IsVariationSelector の処理では2ワード分だけ参照できればよいので、対策としては IsVariationSelector の引数の型を現在の std::string_view から const wchar_t* (＋長さ) に変更しようと思います。

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

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

suconbu · 2024-05-12T09:13:31Z

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

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

suconbu · 2024-05-12T09:14:34Z

sakura_core/charset/codechecker.h

-inline bool IsVariationSelector(std::wstring_view text) {
-	const auto cp = ConvertToUtf32(text);
-	return 0xe0100 <= cp && cp <= 0xe01ef;
+inline bool IsVariationSelector(const wchar_t* pStr, size_t nLen) {


ヌル終端の wchar_t* 型で受け取るので長さ (nLen) をもらう必要なない気がしてきました。(直します)

引数を増やすのは反対です。C言語スタイルのインターフェースが必要ならオーバーロード関数を定義すると良いと思います。

suconbu · 2024-05-12T14:16:45Z

sakura_core/charset/codechecker.h

+ */
+inline bool IsVariationSelector(const wchar_t* pStr, size_t nLen = SIZE_MAX) {
+	return (2 <= nLen) && (pStr[0] == 0xDB40) && (0xDD00 <= pStr[1]) && (pStr[1] <= 0xDDEF);
+}
 /*!
 * 文字列がIVSの異体字セレクタで始まっているか判定する
 */
 inline bool IsVariationSelector(std::wstring_view text) {


今後、std::wstring_view 型で文字列を扱う処理からも使うことも考えて std::wstring_view 型を受け入れる版も引き続き利用できるようにします。

berryzplus · 2024-05-12T13:50:54Z

sakura_core/mem/CNativeW.cpp

@@ -397,7 +397,7 @@ CLogicInt CNativeW::GetSizeOfChar( const wchar_t* pData, int nDataLen, int nIdx
 	}

 	// IVSの異体字セレクタチェック
-	if (IsVariationSelector(pData + nIdx + 1)) {
+	if (IsVariationSelector(pData + nIdx + 1, nDataLen - (nIdx + 1))) {


Suggested change

if (IsVariationSelector(pData + nIdx + 1, nDataLen - (nIdx + 1))) {

if (IsVariationSelector(std::wstring_view(pData + nIdx + 1, nDataLen - (nIdx + 1)))) {

としたら遅延が発生しないということですね。
実質 #1937 の不具合修正ですね。

berryzplus · 2024-05-12T13:55:50Z

sakura_core/charset/codechecker.h

-inline bool IsVariationSelector(std::wstring_view text) {
-	const auto cp = ConvertToUtf32(text);
-	return 0xe0100 <= cp && cp <= 0xe01ef;
+inline bool IsVariationSelector(const wchar_t* pStr, size_t nLen) {


引数を増やすのは反対です。C言語スタイルのインターフェースが必要ならオーバーロード関数を定義すると良いと思います。

berryzplus · 2024-05-12T13:56:46Z

sakura_core/charset/CCodeBase.cpp

@@ -55,7 +55,7 @@ std::wstring CCodeBase::CodeToHex(const CNativeW& cSrc, const CommonSetting_Stat
 EConvertResult CCodeBase::UnicodeToHex(const wchar_t* cSrc, const int iSLen, WCHAR* pDst, const CommonSetting_Statusbar* psStatusbar)
 {
 	// IVS
-	if (iSLen >= 3 && IsVariationSelector(cSrc + 1)) {
+	if (iSLen >= 3 && IsVariationSelector(cSrc + 1, iSLen - 1)) {


発生している問題（不具合？）と関係ない変更に見えます。

berryzplus · 2024-05-12T14:00:39Z

sakura_core/charset/CUtf8.cpp

@@ -227,7 +227,7 @@ EConvertResult CUtf8::_UnicodeToHex(const wchar_t* cSrc, const int iSLen, WCHAR*
 	if (IsUTF16High(cSrc[0]) && iSLen >= 2 && IsUTF16Low(cSrc[1])) {
 		cBuff._GetMemory()->SetRawDataHoldBuffer(cSrc, 4);
 	}
-	else if (iSLen >= 3 && IsVariationSelector(cSrc + 1)) {
+	else if (iSLen >= 3 && IsVariationSelector(cSrc + 1, iSLen - 1)) {


発生している問題（不具合？）と関係ない変更に見えます。

_UnicodeToHexはキャレット位置の文字コードをステータスバーに表示するためのフォーマット関数で、ファイル読み込みで繰り返し呼び出されるものではありません。

残す場合、文字列長は3に決め打ちしてよいはず。

berryzplus · 2024-05-12T14:06:48Z

sakura_core/parse/CWordParse.cpp

@@ -187,7 +187,7 @@ ECharKind CWordParse::WhatKindOfChar(
 	}
 	// IVS（正字 + 異体字セレクタ）
 	else if (nCharChars == 3 &&
-		IsVariationSelector(pData + nIdx + 1))
+		IsVariationSelector(pData + nIdx + 1, pDataLen - (nIdx + 1)))


発生している問題（不具合？）とは無関係に見えます。

が、IVSを大量に含むテキストを開いたときの描画速度に影響する可能性があります。

「せっかくなので合わせて修正」でも「無関係なので捨て」でもどちらでもよいです。

berryzplus · 2024-05-12T14:41:18Z

sakura_core/charset/codechecker.h

+ * 文字列がIVSの異体字セレクタで始まっているか判定する
+ * nLenを省略した時はヌル文字の手前までが有効な文字列であると解釈する
+ */
+inline bool IsVariationSelector(const wchar_t* pStr, size_t nLen = SIZE_MAX) {


Suggested change

inline bool IsVariationSelector(const wchar_t* pStr, size_t nLen = SIZE_MAX) {

inline bool IsVariationSelector(const wchar_t* pStr, size_t nLen) {

これは許容したくありません。（個人の感想です。）

berryzplus · 2024-05-12T14:44:04Z

sakura_core/charset/codechecker.h

+ * nLenを省略した時はヌル文字の手前までが有効な文字列であると解釈する
+ */
+inline bool IsVariationSelector(const wchar_t* pStr, size_t nLen = SIZE_MAX) {
+	return (2 <= nLen) && (pStr[0] == 0xDB40) && (0xDD00 <= pStr[1]) && (pStr[1] <= 0xDDEF);


Suggested change

return (2 <= nLen) && (pStr[0] == 0xDB40) && (0xDD00 <= pStr[1]) && (pStr[1] <= 0xDDEF);

return IsVariationSelector(std::wstring_view(pStr, nLen));

こうしておくとpStr,nLenに不正値が入っていた場合（＝プログラムのバグ）を検出できます。

AppVeyorBot · 2024-05-12T14:57:37Z

✅ Build sakura 1.0.4344 completed (commit b4462a5a14 by @suconbu)

suconbu · 2024-05-12T15:21:16Z

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

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

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

-	CDocFileOperation::FileLoad	うち CLoadAgent::OnLoad	うち CLoadAgent::OnAfterLoad
master (`47d5dc1`)	6487	4025	2425
改善後 (当初案 `fe2e51b`)	3335 (-48%)	2373 (-41%)	927 (-61%)
改善後 (最新 `8df88f1`)	3422 (-47%)	2444 (-39%)	952 (-60%)

sonarcloud · 2024-05-12T15:43:06Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

AppVeyorBot · 2024-05-12T16:22:34Z

✅ Build sakura 1.0.4345 completed (commit 0a84f52504 by @suconbu)

berryzplus · 2024-05-13T03:54:25Z

sakura_core/charset/codechecker.h

@@ -392,8 +392,7 @@ char32_t ConvertToUtf32(std::wstring_view text) {
 * 文字列がIVSの異体字セレクタで始まっているか判定する
 */
 inline bool IsVariationSelector(std::wstring_view text) {
-	const auto cp = ConvertToUtf32(text);
-	return 0xe0100 <= cp && cp <= 0xe01ef;
+	return (2 <= text.size()) && (text[0] == 0xDB40) && (0xDD00 <= text[1]) && (text[1] <= 0xDDEF);


この変更は速度に影響ありますか？

「0xe0100～0xe01ef」が見えなくなることを気にしています。

あります。ファイル読み込み処理全体 (DocFileOperation::FileLoad) の約10%はこの ConvertToUtf32 が消費していました。

suconbu · 2024-05-13T15:42:13Z

レビューありがとうございました。マージします。

suconbu added 2 commits May 11, 2024 15:28

ファイル読み込みに掛かる時間を短縮する

dd0c526

IsVariationSelectorのテストを追加する

fe2e51b

suconbu commented May 11, 2024

View reviewed changes

suconbu changed the title ~~Feature/speedup fileopen~~ ファイル読み込みの高速化 May 11, 2024

kengoide reviewed May 11, 2024

View reviewed changes

suconbu commented May 12, 2024

View reviewed changes

berryzplus reviewed May 12, 2024

View reviewed changes

IsVariationSelectorの引数をstd::wstring_view型に戻す

8df88f1

suconbu force-pushed the feature/speedup_fileopen branch from e9d9d51 to 8df88f1 Compare May 12, 2024 15:32

berryzplus reviewed May 13, 2024

View reviewed changes

berryzplus approved these changes May 13, 2024

View reviewed changes

suconbu merged commit 7eb997a into sakura-editor:master May 13, 2024
24 checks passed

suconbu added the 🚅 speed up 🚀 高速化 label May 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ファイル読み込みの高速化 #1951

ファイル読み込みの高速化 #1951

suconbu commented May 11, 2024 •

edited

suconbu May 11, 2024 •

edited

AppVeyorBot commented May 11, 2024

kengoide left a comment

suconbu commented May 12, 2024

suconbu May 12, 2024

berryzplus May 12, 2024

suconbu May 12, 2024

berryzplus May 12, 2024

berryzplus May 12, 2024

berryzplus May 12, 2024

berryzplus May 12, 2024

berryzplus May 12, 2024

berryzplus May 12, 2024

berryzplus May 12, 2024

AppVeyorBot commented May 12, 2024

suconbu commented May 12, 2024 •

edited

sonarcloud bot commented May 12, 2024

AppVeyorBot commented May 12, 2024

berryzplus May 13, 2024

suconbu May 13, 2024

suconbu commented May 13, 2024

	if (IsVariationSelector(pData + nIdx + 1, nDataLen - (nIdx + 1))) {
	if (IsVariationSelector(std::wstring_view(pData + nIdx + 1, nDataLen - (nIdx + 1)))) {

	inline bool IsVariationSelector(const wchar_t* pStr, size_t nLen = SIZE_MAX) {
	inline bool IsVariationSelector(const wchar_t* pStr, size_t nLen) {

	return (2 <= nLen) && (pStr[0] == 0xDB40) && (0xDD00 <= pStr[1]) && (pStr[1] <= 0xDDEF);
	return IsVariationSelector(std::wstring_view(pStr, nLen));

ファイル読み込みの高速化 #1951

ファイル読み込みの高速化 #1951

Conversation

suconbu commented May 11, 2024 • edited

PR対象

カテゴリ

PR の背景

概要

詳細

1 について

2 について

効果

仕様・動作説明

PR の影響範囲

テスト内容

関連 issue, PR

参考資料

suconbu May 11, 2024 • edited

Choose a reason for hiding this comment

AppVeyorBot commented May 11, 2024

kengoide left a comment

Choose a reason for hiding this comment

suconbu commented May 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AppVeyorBot commented May 12, 2024

suconbu commented May 12, 2024 • edited

sonarcloud bot commented May 12, 2024

Quality Gate passed

AppVeyorBot commented May 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

suconbu commented May 13, 2024

suconbu commented May 11, 2024 •

edited

suconbu May 11, 2024 •

edited

suconbu commented May 12, 2024 •

edited