#398 に対する回避策 (特定のファイルで描画が遅くなる) #792
Conversation
This reverts commit fb81a39555d486d801eeb3488f0f37d795a0d350.
|
掲示板からの転載です。
この PR の Win32/Release 版で試しましたが、数千文字程度から、反応速度があからさまに低下します。URL の色分けチェックを外すと解消します。(ちなみにルーラーの「色分け」チェックもここにあります) wcschr が遅いのではなく、wcschr の呼び出し頻度が非常識なほど多いから速度が低下するんですよね。速度をチューンしても閾値が上がるだけで、テキスト長に応じて再発します。 メールアドレス長はプロトコルの一部なので、明らかに有効でない非常識な長さの文字列をテストする意味はないはずです。そういう方針の修正案を過去に上げました(Git も GitHub も PR の作成方法も知らなかった頃のことです)。 |
|
あ、スラッシュということは、これはメールアドレス判定とは別の問題なのかも。 |
|
別でした(失礼)。掲示板のものは URL 判定に関連した新しい Issue でした。 |
う～む。その通り。
「明らかに有効でない非常識な長さ」が具体的にいくつかが判れば、長さチェックしてFALSEを返すようにすれば問題を回避できる、ということになるんでしょうか。
可能ならPRをお願いしたいっす。 |
|
あ、そ（苦笑ｗ |
取り違えは全然かまわんです。 ぼくはあの書込みをみて、箱型コメント大好き人間なのかな？と邪推しました。 |
|
自分が管理する CSS を確認しましたが、アイコン程度の小さな画像を dataURL にしたものに A が連続するものがありました。それがスラッシュだということは普通にあるのかもしれません。 |
|
掲示板の報告例も結局は IsMailAddress に行き着きました。berryzplus さんのこの PR も、一定の長さで打ち切る方法も、やはりスローダウンしました。 スラッシュに限らず以下の文字種からなる数千以上の長さの文字列で発生します> |
|
整理します。 NG ファイルを開いた際に操作を受け付けなくなるほどの反応速度の低下はこの PR により改善していますが、スローダウンは発生します。旧バージョン(2.0.3.1)では NG ファイルでの無反応もスローダウンもどちらも発生しません。 スローダウンに(も)対応する IsMailAddress より高レベルの対処が TODO になりそうです。遅まきながら、これが根治策だったのではないかと考えています……>「実験した感じだと、行データをスキャンして色替え判定をするあたりで条件ミスマッチが起きているのかな、と思ってます。」 だとしたら罪深い回り道をさせてしまいました。 なぜそう考えるかといえば、「スラッシュに限らず以下の文字種からなる数千以上の長さの文字列で(スローダウンが)発生します> %&'*+-/=?^``{|}~」というリストが、IsMailAddress と _IsPosKeywordHead が利用する文字リストの引き算で出したものだからです。 |
dataURLを誤解してました。バイナリデータをテキスト表現(=base64エンコード)してhtml内に埋め込むやつですか。bit列のパターンによってどの文字が連続してもおかしいことはないですな。 |
This reverts commit 2236d1e.
メールアドレス長の規格値(255字以内)を使って長さチェックする。
if( _IsPosKeywordHead(cStr,nPos) /* URLを表示する */
&& IsURL( cStr.GetPtr() + nPos, cStr.GetLength() - nPos, &nUrlLen ) /* 指定アドレスがURLの先頭ならばTRUEとその長さを返す */
){
this->m_nCOMMENTEND = nPos + nUrlLen;
return true;
}
return false;これは「ある文字列範囲がURLかどうか？」を判定するものです。 根治策として考えていたのは、色替え判定を別スレッドで行うようにすることと判定結果をキャッシュさせることです。この方式なら、メインスレッドは文書全体の色替え判定が終わる前（＝描画が必要な画面領域内の色替え判定が完了した時点）で初回表示を行えるようになります。処理の部分的なスレッド化については「非同期カウントを実装してみた」で試した通り、課題はあるものの実現可能だと思っています。 現時点で行えることは、あくまでworkaround(=暫定対処)です。 とりあえずの対策として「 |
この文章の真意を読み取れてる自信がないっす。想定では打ち切り方式でも多少の改善が見られるはずです。暫定なので「多少」でカバーしきれない分は諦めます。 もし一切改善が見られないとしたらbegincolorの呼び出しに手を入れる必要が… |
|
一定長さで打ち切っても発生するスローダウンというのは、「折り返さない」設定でも折り返してしまう１万文字を少し越える程度の記号の連続を入力する、カーソルを移動させて横スクロールさせる、という状況で発生するものです。 暫定対応策は２つ考えました。
!IS_KEYWORD_CHAR(cStr.At(nPos-1) に加えて IS_KEYWORD_CHAR(cStr.At(nPos) をチェックして、同一の基準で単語境界を判定します。従来は IS_KEYWORD_CHAR(cStr.At(nPos) に相当するチェックは IsURL が担っていましたが、その一部である IsMailAddress が記号対応によりその役目を果たさなくなりました。「ミスマッチ」です。 これにより IsMailAddress の仕様にかかわらず１文字目に許容される記号類が制限されますが、記号対応自体が最近のことでもあり、問題はないと思います。
テストしていませんが、この程度の修正になります>master...ds14050:check_wordboundary_inside_IsMailAddress |
This reverts commit e8a9587.
良さそうだったのでrebaseして積みなおしてみました。（ほぼ丸投げ |
|
掲示板で指摘がありました。 CEditView::IsCurrentPositionURL にも「CColor_Url::BeginColor()と同条件に」とコメントされた同じようなコードがあります。こちらの IsURL も第一引数( |
|
重複コードなんで横展開が必要なのは明らかなんですが、 |
|
性能低下の原因が IsMailAddress に変更を加えた１つのコミットであり、この作業はそのコミットを補完するために IsMailAddress(IsURL) の呼び出し元に目配りするものなのですから、本来的にすべてが一塊の修正であるべきものです。 分かれるのがダメとは言いませんが、分けたい理由はわかりません。 |
|
追加対応を考えますか… 変更を加えた結果遅くなったんだから、影響箇所すべてに手当てがいると考えるのは妥当だと思います。 真因を「速度低下を招く変更を加えた」にしたくないなあと思ってます。 これを真因にしちゃうと、あらゆるコンテキストで実行速度に配慮したコードを書かないといけなくなるし影響確認がめんどくさくなります。 |
|
「速度低下を招く変更を加えた」というよりは「不完全な変更を加えた結果、回避できたはずの速度低下を招いた」と理解しています。 ただ、berryzplus さんの見かたもわかりました。機能としての記号対応はたしかにあれだけで完結したものと見なせます。 しかしだからといって「同根の」速度低下を手当てする対応が分かれることの是非は別です。コミットって対象領域、対象ファイルで分けるものでしょうか。 |
|
セルフツッコミです。
これは反論のための反論だったかもしれません。IsCurrentPositionURL を調べたところ、 sakura/sakura_core/view/CEditView.cpp Line 1285 in 8b3ad82 そうすると IsCurrentPositionURL に関して反応速度の低下は、URL 開始位置における単語境界判定ではなく、IsMailAddress が無制限に先の方までメールアドレスの終端を探してしまうことにあります。 「同根」の速度低下というのは嘘でした。IsCurrentPositionURL に関しては IsMailAddress に以前から内在していた性能問題でした。 |
|
残課題 別件に対処するためのissueを立てる このprに結論を出さないとテストプロジェクトのprを出しづらい… |
There was a problem hiding this comment.
ひょっとして approve 待ちだったでしょうか。自分の役割は済んだような気がしていました。
最初に挙げた「1. CColorStrategy.cpp にある _IsPosKeywordHead (※CColor_Url と CColor_Numeric で使われています) で確実に単語境界を見極める。」の方が数十文字程度の追加で済みそうでおすすめだったのですが、Subversion 時代のコードに対する修正を rebase してしまったんですね……>「Detect wordboundary inside IsMailAddress.」 PoC であり体裁や詰めが気になりますが効果は確認しています。
反応低下の片棒を担いでいた残る課題は、IsMailAddress が入力テキスト長に比例した時間を使ってしまう(ことがある)件です。マウスクリックによるキャレットの移動に対する反応が、長い記号列の先頭近くか末尾近くかで違ってきます。末尾近くの方がアドレス検索の打ち切りが早いために反応が良くなります。
|
レビューありがとうございます。
待ちでした。最近 @beru さんが忙しいみたいなんで、コード読んで意見をいう人が足りないのです。 というわけで、issueを立て次第マージしようと思っています。 |
beru
changed the title
m-tmatma
changed the title
…nd_for_issue398 Feature/workaround for issue398
#398 に対する回避策 (特定のファイルで描画が遅くなる)
reverts #421, and provides workaround for #398.