sakura icon indicating copy to clipboard operation
sakura copied to clipboard

CSVを開いた時の動作が重くなった(2.4.2リリース版)

Open satakagi opened this issue 2 years ago • 13 comments

問題内容

Ver2.4.2でCSVを開くと、それ以前の版と比べて動作が重くなりました

再現手順

添付画像の設定をしたあと、たとえば、こちらのCSVを開いてみます。 https://www.npa.go.jp/publications/statistics/koutsuu/opendata/koudohyou/9_koudohyou_rosen_kousokujidousya_jidousyasenyou.csv

再現頻度

カラム数が大きめのCSVで発生しているようです。

問題のカテゴリ

  • プログラムの動作上の問題
    • 正式リリース版

環境情報

  • OS バージョン Windows11 Pro x86
  • サクラエディタバージョン Ver. 2.4.2.6048

スクリーンショット

image

satakagi avatar Feb 24 '23 01:02 satakagi

Ver2.4.2でCSVを開くと、それ以前の版と比べて動作が重くなりました

どれくらい遅くなってますか? 自分の環境では1秒もかからなかったので再現確認できませんでした。

目安として「5秒以上かかるようなら要検討かなぁ」と。 比較して倍以上かかるのもマズいと思います。

自分の環境はこんなんです。

プロセッサ 11th Gen Intel(R) Core(TM) i7-1185G7 @ 3.00GHz 3.00 GHz 実装 RAM 32.0 GB (31.7 GB 使用可能) システムの種類 64 ビット オペレーティング システム、x64 ベース プロセッサ

berryzplus avatar Feb 24 '23 15:02 berryzplus

自分もCSVファイルをダウンロードして設定して読み込んでCSV表示してみたんですが、どこらへんの動作が重くなったのか良く分からないです。

beru avatar Feb 25 '23 18:02 beru

カラム数が大きめのCSVで発生しているようです。

Azure MonitorのtracesログをエクスポートしたCSVファイルを開いて試してみました。

  • tracesログは31カラムあります。
  • カラムの区切りを正しく認識できない 事象を確認しました。
  • 横スクロール時に固まる 事象を確認しました。
  • ファイルオープン時の操作遅延があるかどうかは判別できませんでした。

確認した2件の事象はいずれも不具合のように見えますが、 横スクロール時に固まる はv2.4.1では発生していなかったようです。

berryzplus avatar Feb 26 '23 16:02 berryzplus

9_koudohyou_rosen_kousokujidousya_jidousyasenyou.zip

9_koudohyou_rosen_kousokujidousya_jidousyasenyou.csv ファイルの列を複製して512列にしたcsvファイルを読んでみましたが、動作が重いという現象は確認出来ませんでした。

ただEndキーを押して行末に移動するとソフトが終了してしまう不具合を確認しました。デバッグして原因を調べてみると、CRuler::DrawRulerBg メソッドで nMaxLineKetas (10240) より keta の値が大きくなる為に結果的に負の値が size_t 型のローカル変数に代入され、極端に大きい値が std::vector::resize メソッドに渡されて落ちている事が分かりました。

screenshot

image

beru avatar Mar 05 '23 09:03 beru

2.4.2のインストーラ版を使用していますが、CSV・TSVのファイルオープン、スクロールが極端に遅くなる現象がこちらでも発生しています。 2.4.1のインストーラ版では起きていません。 再現するCSVファイルを作成しましたので添付しました。 hogehoge.csv どうやら、文字列の少ないセルと文字列の多いセルが入り混じっていると起きるようです。

astanabe avatar Apr 10 '23 22:04 astanabe

2.4.2のインストーラ版を使用していますが、CSV・TSVのファイルオープン、スクロールが極端に遅くなる現象がこちらでも発生しています。 2.4.1のインストーラ版では起きていません。 再現するCSVファイルを作成しましたので添付しました。 hogehoge.csv どうやら、文字列の少ないセルと文字列の多いセルが入り混じっていると起きるようです。

デバッグしてみたところ、CFigure_Comma::DispSpaceExtTextOut を呼び出す際に szViewString の文字数が 64 を超過すると pMetrics->GetDxArray_AllHankaku() で取得する領域の範囲外参照が発生する為問題に繋がるようです。 link

CTextMetrics::m_anHankakuDx の型は std::array<int, 64> なので。 link

beru avatar Apr 11 '23 01:04 beru

範囲外参照は致命的ですね…。

m_anHankakuDx の中身は固定値です。余白+コンマが64文字を超える場合は、バッファをその場で用意してその場で同値を書き込むように変更すれば範囲外参照は解決できます。 https://github.com/sakura-editor/sakura/blob/9e8fa57da5823fc394a10dd666bc30b36a4b9167/sakura_core/view/CTextMetrics.cpp#L65

今週末まで私用で動けないので来週に解消PRを出します(先に動いてくださる方がいらっしゃれば急ぎでレビューします)。

kengoide avatar Apr 11 '23 08:04 kengoide

63文字までしか想定してない関数に 64文字以上渡した場合にメモリ確保するんです?

というツッコミをPR書いてしまう前に入れときます。

berryzplus avatar Apr 11 '23 13:04 berryzplus

ExtTextOut の最後の引数 lpDX は optional で NULL の場合にはデフォルトの文字間隔が使われるようです。デフォルトの文字間隔が何で設定されるのか良く分かっていませんが、 SetTextCharacterExtra 関数で設定できる値の事でしょうか?(DCに関連する)グローバル設定なのでお手軽ですが影響範囲が広いので使う場合は色々確認が必要ですね。

beru avatar Apr 12 '23 12:04 beru

試しに CFigure_Comma::DispSpaceExtTextOut を呼び出す際の lpDx 引数を NULL に変えても特に問題は無さそうです。描画する文字列の内容が , の後に半角空白を追加しているだけだからですかね?

beru avatar Apr 12 '23 12:04 beru

ExtTextOutで幅を指定しないとプロポーショナルフォント使用時にCSVモードの桁が揃わなくなるのかな?と想像していました。問題がなさそうならNULL指定で対処するほうが良いですね。

kengoide avatar Apr 12 '23 12:04 kengoide

ExtTextOutで幅を指定しないとプロポーショナルフォント使用時にCSVモードの桁が揃わなくなるのかな?と想像していました。問題がなさそうならNULL指定で対処するほうが良いですね。

NULL指定ではなくて、試しに m_anHankakuDx の型を std::array<int, 512> に変えて確認しました。

プロポーショナルフォントを使用した場合

image

プロポーショナルフォントを使用しない場合

image

桁が揃わないという事は無いですが、なんというか広すぎますね。これはこれで別の不具合ですかね。。

なおNULL指定でも表示は同じじゃないかと思います。厳密に比較はしていませんが…。

beru avatar Apr 12 '23 12:04 beru

なんとなく思っていること

  • CEditViewにCSVモードが実装されているのは誤りなのではないか?
    • CEditView の役割 ≒ プレーンテキストを表示して編集できるUIを提供する。
    • CSVモードの役割 ≒ CSV(Character Separated Values)を表示して編集できるUIを提供する。

これは一般的なオブジェクト指向設計の指針「単一責務の原則 Single Responsibility principal 」に反している気がします。

もっとも、サクラエディタは基本的にC言語で書かれたプログラムなので、オブジェクト指向言語の設計指針に従っていないからと言って一概に「誤りである」とも言えないような気はします。

で、どうするの?

妥当性を検証できる回答は提示できません。

これまで通り「やりたい人がやりたいように提案する」で進めて行ったらよいと思います。

berryzplus avatar Apr 14 '23 03:04 berryzplus