sakura
sakura copied to clipboard
sakura-editor
共有メモリの文字幅キャッシュを構築するタイミングがおかしい
問題内容
タイトル通りです。
共有メモリ上に文字幅キャッシュを構築する処理が、 取得した共有メモリが使えるかどうかをチェックする判定の「前」にあります。
https://github.com/sakura-editor/sakura/blob/e013bcdce61de61bef8fd9c5e0e5abd63bb3c962/sakura_core/env/CShareData.cpp#L741-L747
キャッシュというのは普通、作成するのにある程度時間のかかるデータを何度も作成することによる時間のロスを防ぐために使うものなので、チェック判定で途中終了させるロジックがある場合は「チェックの後」に作成するようにすべきと思います。
他にもたくさんあるんですけど、これが一番分かりやすい問題だと思うので一応issue立てときます。
再現手順
再現頻度
論理的な処理順序の不備ですので常に再現します。 ただし、影響を受けるユーザは以下すべてを満たす人のみで、開発者に限られると思います。
- PC内に異なるバージョンのサクラエディタを持っている。
- 起動中のサクラエディタと異なるバージョンのサクラエディタを「手動で」起動させた。
問題のカテゴリ
- その他の問題
関連する不具合
#1523
環境情報
- OS バージョン
- サクラエディタバージョン
- PC情報
スクリーンショット
確かに不要な計算が行われる可能性のあるコードですね。
今ある文字幅キャッシュをプロセス間で共有するという仕組みは得られる利益とコードの複雑性が釣り合ってない気がしていて、できればばっさり削除してしまいたいと思っています。ブランチは手元にあるのですが、性能の変化の検証をしてPRにまとめる作業に手を付ける気にならず放置中です…。
ブランチは手元にあるのですが、性能の変化の検証をしてPRにまとめる作業に手を付ける気にならず放置中です…。
あら。。。
- 関連PR出したら競合するのかしら？ 競合するでしょうね・・・
- 文字幅キャッシュの構築ってそもそもどれくらいかかるのかしら？ そんなに時間はかかってないでしょうね・・・
- 文字場キャッシュの構築ってそもそも必要なのかしら？ 文字幅計算の対象になる文字数が多く、かつ、重複する文字が多い場合には効果あるでしょうね・・・
文字幅キャッシュに関して気付いた他の問題
- 文字幅キャッシュを作成する必要があるかどうかを判定していない
- MapViewOfFileが失敗した場合が考慮されていない
- 文字幅キャッシュの構築はエディタプロセス側だけで必要な処理だが、コード上その意図が読み取りづらく「そもそもここで構築するのが不適切なんじゃね？」という本質的なギモンに辿り着きにくい