sakura
sakura copied to clipboard
CSVを開いた時の動作が重くなった(2.4.2リリース版)
問題内容
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
スクリーンショット
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 ベース プロセッサ
自分もCSVファイルをダウンロードして設定して読み込んでCSV表示してみたんですが、どこらへんの動作が重くなったのか良く分からないです。
カラム数が大きめのCSVで発生しているようです。
Azure MonitorのtracesログをエクスポートしたCSVファイルを開いて試してみました。
- tracesログは31カラムあります。
- カラムの区切りを正しく認識できない 事象を確認しました。
- 横スクロール時に固まる 事象を確認しました。
- ファイルオープン時の操作遅延があるかどうかは判別できませんでした。
確認した2件の事象はいずれも不具合のように見えますが、 横スクロール時に固まる はv2.4.1では発生していなかったようです。
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
2.4.2のインストーラ版を使用していますが、CSV・TSVのファイルオープン、スクロールが極端に遅くなる現象がこちらでも発生しています。 2.4.1のインストーラ版では起きていません。 再現するCSVファイルを作成しましたので添付しました。 hogehoge.csv どうやら、文字列の少ないセルと文字列の多いセルが入り混じっていると起きるようです。
2.4.2のインストーラ版を使用していますが、CSV・TSVのファイルオープン、スクロールが極端に遅くなる現象がこちらでも発生しています。 2.4.1のインストーラ版では起きていません。 再現するCSVファイルを作成しましたので添付しました。 hogehoge.csv どうやら、文字列の少ないセルと文字列の多いセルが入り混じっていると起きるようです。
デバッグしてみたところ、CFigure_Comma::DispSpace
で ExtTextOut
を呼び出す際に szViewString
の文字数が 64 を超過すると pMetrics->GetDxArray_AllHankaku()
で取得する領域の範囲外参照が発生する為問題に繋がるようです。 link
CTextMetrics::m_anHankakuDx
の型は std::array<int, 64>
なので。 link
範囲外参照は致命的ですね…。
m_anHankakuDx
の中身は固定値です。余白+コンマが64文字を超える場合は、バッファをその場で用意してその場で同値を書き込むように変更すれば範囲外参照は解決できます。
https://github.com/sakura-editor/sakura/blob/9e8fa57da5823fc394a10dd666bc30b36a4b9167/sakura_core/view/CTextMetrics.cpp#L65
今週末まで私用で動けないので来週に解消PRを出します(先に動いてくださる方がいらっしゃれば急ぎでレビューします)。
63文字までしか想定してない関数に 64文字以上渡した場合にメモリ確保するんです?
というツッコミをPR書いてしまう前に入れときます。
ExtTextOut
の最後の引数 lpDX
は optional で NULL の場合にはデフォルトの文字間隔が使われるようです。デフォルトの文字間隔が何で設定されるのか良く分かっていませんが、 SetTextCharacterExtra
関数で設定できる値の事でしょうか?(DCに関連する)グローバル設定なのでお手軽ですが影響範囲が広いので使う場合は色々確認が必要ですね。
試しに CFigure_Comma::DispSpace
で ExtTextOut
を呼び出す際の lpDx
引数を NULL に変えても特に問題は無さそうです。描画する文字列の内容が ,
の後に半角空白を追加しているだけだからですかね?
ExtTextOutで幅を指定しないとプロポーショナルフォント使用時にCSVモードの桁が揃わなくなるのかな?と想像していました。問題がなさそうならNULL指定で対処するほうが良いですね。
ExtTextOutで幅を指定しないとプロポーショナルフォント使用時にCSVモードの桁が揃わなくなるのかな?と想像していました。問題がなさそうならNULL指定で対処するほうが良いですね。
NULL指定ではなくて、試しに m_anHankakuDx
の型を std::array<int, 512>
に変えて確認しました。
プロポーショナルフォントを使用した場合
プロポーショナルフォントを使用しない場合
桁が揃わないという事は無いですが、なんというか広すぎますね。これはこれで別の不具合ですかね。。
なおNULL指定でも表示は同じじゃないかと思います。厳密に比較はしていませんが…。
なんとなく思っていること
- CEditViewにCSVモードが実装されているのは誤りなのではないか?
- CEditView の役割 ≒ プレーンテキストを表示して編集できるUIを提供する。
- CSVモードの役割 ≒ CSV(Character Separated Values)を表示して編集できるUIを提供する。
これは一般的なオブジェクト指向設計の指針「単一責務の原則 Single Responsibility principal 」に反している気がします。
もっとも、サクラエディタは基本的にC言語で書かれたプログラムなので、オブジェクト指向言語の設計指針に従っていないからと言って一概に「誤りである」とも言えないような気はします。
で、どうするの?
妥当性を検証できる回答は提示できません。
これまで通り「やりたい人がやりたいように提案する」で進めて行ったらよいと思います。