特定のファイルで描画が遅くなる。

[KENCHjp]

２万強の改行無しファイルで、やたら描画が遅くなる場合がある。

大丈夫な方。
testOK23000.txt

ダメな方。
testNG23000.txt

[KENCHjp]

ちなみにメモ帳ではサクッと開ける。

[berryzplus]

なんとなく、utf-7を判定しにいって重くなってそうな気配を感じました。

https://ja.wikipedia.org/wiki/UTF-7

ためしに重い方にBOMを付けて保存してみたけど、あまり変わらず・・・ｗ

逆の発想で、軽いほうに + を付けてみたが、やはり変わらず・・・ｗ

「じゃあこれだっ！」とばかりに s|23|2///3|g で一括置換してみました。
ビンゴです。これだけで、軽い方のファイルもだいぶ重くなります。

URL周りの判定はかなり重そうなので気にはなっていました。
体感速度にかなり差がでる部分なので「いつか改善すべき箇所」として認識しました。
このあたり、結構見てるのでレビューなら協力できると思います。

[ds14050]

2.3.0.0 は NG ファイルを普通に扱えました。
2.3.1.0 は 2.3.2.0 と同じ様に異常に時間がかかりました。

この間に berryzplus さんの言うところと関係する変更があったのでしょうか。

[m-tmatma]

2.3.0.0 は NG ファイルを普通に扱えました。
2.3.1.0 は 2.3.2.0 と同じ様に異常に時間がかかりました。

前に動いていたのなら git bisect しましょう。
参考:
https://qiita.com/usamik26/items/cce867b3b139ea5568a6
#326 (comment)

[berryzplus]

2.3.0.0 は NG ファイルを普通に扱えました。
2.3.1.0 は 2.3.2.0 と同じ様に異常に時間がかかりました。

なんと！昔から遅かったと思っていました。

実は、コードチェックはまだあんまりしてないのです。
実験した感じだと、行データをスキャンして色替え判定をするあたりで条件ミスマッチが起きているのかな、と思ってます。

[ds14050]

前に動いていたのなら git bisect しましょう。

はい。以前はそれを手作業でやっていたらしいことと合わせて、コマンドは聞いたことがあります。しかしなにぶんローカルビルド環境が……。OS の入手から……。

代わりに Very Sleepy で何をやっているかを見てみました。
IsURL から呼ばれる IsMailAddress から呼ばれる wcschr がほとんどの時間を消費しているらしいです。この部分です

sakura/sakura_core/parse/CWordParse.cpp

Lines 458 to 468 in 8fb936b

	while( j < nBufLen - 2 &&
	(
	(pszBuf[j] >= L'a' && pszBuf[j] <= L'z')
	|| (pszBuf[j] >= L'A' && pszBuf[j] <= L'Z')
	|| (pszBuf[j] >= L'0' && pszBuf[j] <= L'9')
	|| (pszBuf[j] == L'.')
	|| NULL != wcschr(L"!#$%&'*+-/=?^_`{|}~", pszBuf[j])
	)
	){
	j++;
	}

ということはこれですか>「Chg: メールアドレスの記号類を許可する · sakura-editor/sakura@f6990fd」

＃ Very Sleepy 以降の流れには検証が必要です。
＃ 毛利小五郎ばりにテキトーにほいほいと犯人を決めつけにかかっています。
＃ 検証にはたとえば git bisect など……。

[berryzplus]

＃ 毛利小五郎ばりにテキトーにほいほいと犯人を決めつけにかかっています。

誰もが工藤新一にはなれんです。小五郎さんでもいいじゃないですか。
ちなみに「不用意に wcschr を導入したこと」を原因の１つと見ることに異論はないです。

が、関数の仕様として正しい変更であるようにもぼくには見えています。
原因は、この修正を行ったことそのものじゃなくて、
この速いとはいえない関数が ~~~20万回~~~ 2万回 よばれることなんじゃないかなぁ、とか。
（ファイルサイズは22Kなので2万回でした。色替え判定は一文字ずつなので2万回です。）

前にどっかでも同じようなことを書きました。
遅いことが分かっているなら呼ばなきゃいいのです。

この変更により関数の実行速度が落ちることに修正した人が気付かなかったとは考えにくいので、「呼出ルートの確認漏れ」という人為的ミスによる不具合の可能性が出てきたと思っています。

お互いにレビューし合ってミスを減らせる環境があったなら防げたものなのかも知れません。

うーむ。

[berryzplus]

vs2017のプロファイラで確認してみました。

テスト内容

1. sakura.exe 起動(無題2の編集ウインドウが開く)
2. testNG23000.txt を開く
3. testNG23000.txt を閉じる(プロセス終了)

※前提として sakura.exe の管理プロセスが起動した状態にして実験しました。

テスト結果

1. wcschr (42.92%)
2. IsMailAddress (24.42%)
3. CheckChangeColor (0.02%)

時間の掛かってるもの上位3件です。
括弧内数字はテスト全体に占める時間割合だと思ってください。
もっともCPU時間を多く消費しているのは @ds14050 さんが引用されているコードの464行目でした。

検証実験として以下のようにコードを書き換えてビルドすると
NGファイルもサクッと開けるようになりました。

//	 || NULL != wcschr(L"!#$%&'*+-/=?^_`{|}~", pszBuf[j])
	 || (pszBuf[j] == L'!')
	 || (pszBuf[j] == L'#')
	 || (pszBuf[j] == L'$')
	 || (pszBuf[j] == L'%')
	 || (pszBuf[j] == L'&')
	 || (pszBuf[j] == L'\'')
	 || (pszBuf[j] == L'*')
	 || (pszBuf[j] == L'+')
	 || (pszBuf[j] == L'-')
	 || (pszBuf[j] == L'/')
	 || (pszBuf[j] == L'=')
	 || (pszBuf[j] == L'?')
	 || (pszBuf[j] == L'^')
	 || (pszBuf[j] == L'_')
	 || (pszBuf[j] == L'`')
	 || (pszBuf[j] == L'{')
	 || (pszBuf[j] == L'|')
	 || (pszBuf[j] == L'}')
	 || (pszBuf[j] == L'~')

しかし、さすがにこれは汚くね？と思うコードなのでPRするのはやや微妙。

本質的には初回レイアウト処理を見直したほうがよいと思ってるのでここだけ直すのはやや微妙。

ただ、この修正だけで許容できる速度を取り戻せることを確認してしまったので直さないのも微妙。

う～む。

[KENCHjp]

運用に耐えるようにすることは、綺麗なコードを書くよりも優先する事かと。

[m-tmatma]

この pszBuf[j] に対する判定を全て、一つのswitch で判定するようにしたら更に速くなりませんか？

sakura/sakura_core/parse/CWordParse.cpp

Lines 458 to 468 in 8fb936b

	while( j < nBufLen - 2 &&
	(
	(pszBuf[j] >= L'a' && pszBuf[j] <= L'z')
	|| (pszBuf[j] >= L'A' && pszBuf[j] <= L'Z')
	|| (pszBuf[j] >= L'0' && pszBuf[j] <= L'9')
	|| (pszBuf[j] == L'.')
	|| NULL != wcschr(L"!#$%&'*+-/=?^_`{|}~", pszBuf[j])
	)
	){
	j++;
	}

[ds14050]

制限して良ければ長さ制限をかけた方が効果的でした(もちろん wcschr の置き換えと併用すればさらに効果があるでしょう)。テキストの長さとマシンの速さに依存しないところがメリットです。

faster_ismailaddress.txt

[berryzplus]

この pszBuf[j] に対する判定を全て、一つのswitch で判定するようにしたら更に速くなりませんか？

速くなると思います。

詳細忘れましたが、分岐数と被判定値の下限＆上限が一定の条件を満たせば、
if文を並べるよりもswitch文にした方が速くなったはずです。
(生成コードが table jump だとswitchのが速い、2分探索だと条件によってはifのが速い。)

ところで、 @ds14050 さんの提示コードを見て思ったんですが、
利用可能なメールアドレスの「アカウント名」ってどんなんでしたっけ？

メールアドレス：：＝ アカウント名 "＠" メールサーバー名

コードでは「 "." で始まらない限りどんなものでも有効」と読めたけど本当？

[KENCHjp]

https://wrangler-jeans.jp/shop/pages/Faq_Member_RFC.aspx

このへんかな。

[beru]

検証実験として以下のようにコードを書き換えてビルドすると
NGファイルもサクッと開けるようになりました。

//	 || NULL != wcschr(L"!#$%&'*+-/=?^_`{|}~", pszBuf[j])
	 || (pszBuf[j] == L'!')
ry

しかし、さすがにこれは汚くね？と思うコードなのでPRするのはやや微妙。

インライン関数を作成してその中に閉じ込めてしまえばあまり気にならないと思います。
あと事前に 7bit の範囲内の判定用にテーブルを作って判定に利用する方法でも処理時間を削減できると思います。

[berryzplus]

利用可能なメールアドレスの「アカウント名」ってどんなんでしたっけ？
メールアドレス：：＝ アカウント名 "＠" メールサーバー名

https://wrangler-jeans.jp/shop/pages/Faq_Member_RFC.aspx
このへんかな。

なんか歴史的な理由で、RFC非準拠のメールアドレスを使っている人がいるようなので、
完全に RFC準拠 で判定してはいけないような雰囲気を感じました。
取り入れたら判定が速くなりそうな縛りがけっこうあるんですけど。

• @手前は記号で始まってはいけない
• @手前は記号で終わってはいけない
• @手前は連続する記号のシーケンスを含んではいけない
• @ 手前は63字以内
• 全体で255字以内

[ds14050]

付加機能の例外対応のためにテキスト編集の実用性を犠牲にするのは本末転倒気味なので、長さ制限は入れたいです。

[KENCHjp]

なんか歴史的な理由で、RFC非準拠のメールアドレスを使っている人がいるようなので、

某OSを作ってる会社が作ったメーラーと、そのメールサーバーがRFCをことごとく無視していた昔があったような古の記憶が・・・

[KENCHjp]

ちな、Gmailだと、

けんち@サクラエディタ.jp

なんてアドレスでも送れちゃうので、いっそ

mailto:〜@〜.〜 [空白]

でも私は困らない（笑）

[berryzplus]

土日使ってPR作ってみます。

[beru]

こちらでも f972a2f で用意していただいた testNG23000.txt を開いて再現しました。ひー重い…。

[beru]

#421 を Merge して問題が解消したので Close します。