Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
satakagi opened this issue Feb 24, 2023 · 13 comments
Open

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

satakagi opened this issue Feb 24, 2023 · 13 comments
Labels
🐛bug🦋 ■バグ修正(Something isn't working) 💩degradation🧻🚽 デグレ (前に動いていた機能が動かなくなった)

Comments

@satakagi
Copy link

問題内容

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

@berryzplus
Copy link
Contributor

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 ベース プロセッサ

@beru
Copy link
Contributor

beru commented Feb 25, 2023

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

@berryzplus
Copy link
Contributor

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

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

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

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

@beru
Copy link
Contributor

beru commented Mar 5, 2023

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

@astanabe
Copy link

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

@beru
Copy link
Contributor

beru commented Apr 11, 2023

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

@kengoide
Copy link
Member

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

m_anHankakuDx の中身は固定値です。余白+コンマが64文字を超える場合は、バッファをその場で用意してその場で同値を書き込むように変更すれば範囲外参照は解決できます。

std::fill(m_anHankakuDx.begin(), m_anHankakuDx.end(), m_nDxBasis);

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

@berryzplus
Copy link
Contributor

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

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

@beru
Copy link
Contributor

beru commented Apr 12, 2023

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

@beru
Copy link
Contributor

beru commented Apr 12, 2023

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

@kengoide
Copy link
Member

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

@beru
Copy link
Contributor

beru commented Apr 12, 2023

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

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

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

image

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

image

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

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

@berryzplus
Copy link
Contributor

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

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

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

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

で、どうするの?

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

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

@beru beru added 🐛bug🦋 ■バグ修正(Something isn't working) 💩degradation🧻🚽 デグレ (前に動いていた機能が動かなくなった) labels Sep 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛bug🦋 ■バグ修正(Something isn't working) 💩degradation🧻🚽 デグレ (前に動いていた機能が動かなくなった)
Projects
None yet
Development

No branches or pull requests

5 participants