CDocLine の容量削減

[beru]

PR の目的

CDocLine のインスタンスのサイズを削減する事でメモリ使用量を抑える

カテゴリ

• その他

PR の背景

行数の分だけ CDocLine のインスタンスが作成されるので行数が非常に多いファイルを開いたりするとその分だけ大量にメモリを使用します。

このPRでは x64 Release ビルドの場合に sizeof(CDocLine) を 56 から 40 に削減しました。インスタンス１つにつき16バイト減っているので16777216行あるファイルを開いた場合に消費されるメモリ使用量が256MiB削減される事になります。

どのようなやり方で CDocLine のサイズを削減したかについて下記に記載します。

容量を節約する為に一部の列挙型のサイズを1バイトにしたり、CDocLine クラスのメンバーの順序を入れ替えたりパディングを無しにして詰めました。

またメモリバッファクラス（CMemory）の容量を減らす為にデストラクタを仮想ではないように変更しました。仮想関数を持つとインスタンスに仮想関数テーブルへのポインタの隠しメンバがコンパイラによって自動的に挿入されるのでそれを避ける為です。メモリバッファクラスとその派生クラスはポリモーフィズムは使用していない為、不要と判断して削除しました。

CNativeW クラスにデバッグビルド時にデバッグ用のメンバー m_pDebugData が追加されていましたが、ウォッチウィンドウで書式指定すれば同じ事が実現出来るので不要と判断しました。

PR のメリット

メモリ使用量が減ります。

PR のデメリット (トレードオフとかあれば)

詰めて記録するようにした事により処理速度にわずかに悪影響が出るかもしれません（未確認）。

PR の影響範囲

CMemory, CNative, CNativeW 関連

CDocLine 関連

関連チケット

#985 ではメモリプールを使っていますがその変更もメモリ使用量を減らす効果が有ります。

[AppVeyorBot]

✅ Build sakura 1.0.2120 completed (commit 008665cdda by @)

[AppVeyorBot]

✅ Build sakura 1.0.2121 completed (commit 8911afee50 by @beru)

[berryzplus]

容量を減らすための方法に納得がいっていないです。
CDoclineの容量を削減したい、とか、その必要がありそう、ってとこには同意です。

enumの宣言型をcharに変えてるんですけど、8bit整数の判定より32bit整数の判定のが速いはずです。
影響を構造体からの取り出し時のみに抑えられるように ビットフィールド で代替するって手が使える気がします。

数値、構造体、数値だとアラインメントがうまく行かんはずなので、メンバ入替をするのは問題ない認識です。

やや別件寄りな話になってしまいますけど、CEolがCDocLineクラスの一部になってることには違和感を感じています。何故かというとCEolは改行コードを画面表示するためのものだからです。サクラエディタには、内部的な行データを表すクラスが2つありますが、CDocLineはデータ寄りのクラスです。画面表示のための情報をデータ寄りのクラスが持っているのはおかしいと思います。

CMemoryの仮想デストラクタを削るのは反対です。
仮想デストラクタを削ってしまうと、劣化版スマートポインタとしてのCMemoryの役割が果たせなくなります。（つまり、変数がスコープを抜けるときに手動で解放処理しないといけなくなる）

CMemory(派生クラスを含む)を廃止したい、であれば賛成できる余地がありますが、そのためには準備作業が必要だと思っていて、現状では賛同できません。

CNativeWのデバッグポインタは、natvisを使って代替できます。
前にビルドエラーの原因になったときに、現代的手法への差し替えを提案したことがあります。

このPRの対応だと単にデバッグ変数を削ってるだけなのであんまり良くない気がします。

[beru]

enumの宣言型をcharに変えてるんですけど、8bit整数の判定より32bit整数の判定のが速いはずです。

速度については仮にこのPRで低下したとしても（計測してないのでどう変化したか良く分かりませんが）他の変更でリカバリー出来れば良いと考えています。

影響を構造体からの取り出し時のみに抑えられるように ビットフィールド で代替するって手が使える気がします。

自分も最初ビットフィールド指定が使えるかと思ったんですが、実際に指定してみると下記のエラーメッセージが出ました。

1>d:\projects\sakura\sakura_core\doc\logic\cdocline.h(108): error C2150: 'CDocLine::MarkType::m_cBookmarked': bit field must have type 'int', 'signed int', or 'unsigned int' (compiling source file ..\sakura_core\CAutoSaveAgent.cpp)

ビットフィールド指定出来る型は限られているようです。

容量を節約するにはビット単位で詰めた方がより縮められるのは確かで、拡張情報の個々のメンバをクラスにしないであと行末コードについても構造体に含めて扱うのが容量の節約の為には良いと思います。しかしその変更を行うと差分の量が激しくなるので今回は実施しませんでした。

数値、構造体、数値だとアラインメントがうまく行かんはずなので、メンバ入替をするのは問題ない認識です。

#pragma pack(push,1) と #pragma pack(pop) で class CDocLine の宣言を囲んでパディング無しにしたので、もしかしたらメンバの順番の入れ替えは必ずしも必要では無いかもしれません。

やや別件寄りな話になってしまいますけど、CEolがCDocLineクラスの一部になってることには違和感を感じています。何故かというとCEolは改行コードを画面表示するためのものだからです。サクラエディタには、内部的な行データを表すクラスが2つありますが、CDocLineはデータ寄りのクラスです。画面表示のための情報をデータ寄りのクラスが持っているのはおかしいと思います。

バッファ（文字列データの）中にも改行コードは持っているようなので別のメンバとしても抱えているのは実装を単純化する為なんですかねぇ？

CMemoryの仮想デストラクタを削るのは反対です。
仮想デストラクタを削ってしまうと、劣化版スマートポインタとしてのCMemoryの役割が果たせなくなります。（つまり、変数がスコープを抜けるときに手動で解放処理しないといけなくなる）

CMemory(派生クラスを含む)を廃止したい、であれば賛成できる余地がありますが、そのためには準備作業が必要だと思っていて、現状では賛同できません。

「劣化版スマートポインタとしてのCMemory」ってのがよくわかりません。コード例で示してもらう事は出来ますか？

一般的にデストラクタを virtual にするのは派生クラスを動的に確保したものを後で解放する際に 基底クラスのポインタ経由で行うと派生クラスのデストラクタが呼ばれないのが問題になります。
でもそういうところってありますか？

あと現時点では派生クラスの CNative にも CNativeW にもデストラクタ追加していないし基本的にメンバー追加もしていない事から問題無いのではないでしょうか？

CNativeWのデバッグポインタは、natvisを使って代替できます。
前にビルドエラーの原因になったときに、現代的手法への差し替えを提案したことがあります。

このPRの対応だと単にデバッグ変数を削ってるだけなのであんまり良くない気がします。

.natvis は使いたければ各自で整備してもらえば良い事だと思います。

[AppVeyorBot]

✅ Build sakura 1.0.2139 completed (commit 93b545782b by @beru)

[berryzplus]

enumの宣言型をcharに変えてるんですけど、8bit整数の判定より32bit整数の判定のが速いはずです。

速度については仮にこのPRで低下したとしても（計測してないのでどう変化したか良く分かりませんが）他の変更でリカバリー出来れば良いと考えています。

この件は同意。
ただ、変える必要があるのかないのか？は気にしたいと思っています。

• 必要なければ変えないがいいです。
• 必要だから変えるにしたいです。

影響を構造体からの取り出し時のみに抑えられるように ビットフィールド で代替するって手が使える気がします。

自分も最初ビットフィールド指定が使えるかと思ったんですが、実際に指定してみると下記のエラーメッセージが出ました。

1>d:\projects\sakura\sakura_core\doc\logic\cdocline.h(108): error C2150: 'CDocLine::MarkType::m_cBookmarked': bit field must have type 'int', 'signed int', or 'unsigned int' (compiling source file ..\sakura_core\CAutoSaveAgent.cpp)

ビットフィールド指定出来る型は限られているようです。

指定できないなら仕方ないですな 😢

容量を節約するにはビット単位で詰めた方がより縮められるのは確かで、拡張情報の個々のメンバをクラスにしないであと行末コードについても構造体に含めて扱うのが容量の節約の為には良いと思います。しかしその変更を行うと差分の量が激しくなるので今回は実施しませんでした。

容量を節約するために型に手を入れようとしているメンバが「すべてpublicな件」も関係してそうな気がします。ぼく個人は、得意な言語がほぼOOPなこともあり、基本的にオブジェクト思考です。全メンバがpublicだと設計をサボったうん○クラスに見えてしまいます。仮にメンバがカプセル化されていたなら、内部実装をintに変更してビットフィールドにしても影響は出ないわけで:smile:

数値、構造体、数値だとアラインメントがうまく行かんはずなので、メンバ入替をするのは問題ない認識です。

#pragma pack(push,1) と #pragma pack(pop) で class CDocLine の宣言を囲んでパディング無しにしたので、もしかしたらメンバの順番の入れ替えは必ずしも必要では無いかもしれません。

え、そうなの？
char, struct {int}, charの並びで整列させると
char, padding, struct {int}, char になりそうな気がしますが・・・（←試してない

やや別件寄りな話になってしまいますけど、CEolがCDocLineクラスの一部になってることには違和感を感じています。何故かというとCEolは改行コードを画面表示するためのものだからです。サクラエディタには、内部的な行データを表すクラスが2つありますが、CDocLineはデータ寄りのクラスです。画面表示のための情報をデータ寄りのクラスが持っているのはおかしいと思います。

バッファ（文字列データの）中にも改行コードは持っているようなので別のメンバとしても抱えているのは実装を単純化する為なんですかねぇ？

完全に別件な話になってしまいますので別件で具体的に話すのが良さそうです。
CDocLineにCEolが含まれることによって、むしろ複雑な実装になってるように思っています。

CMemoryの仮想デストラクタを削るのは反対です。
仮想デストラクタを削ってしまうと、劣化版スマートポインタとしてのCMemoryの役割が果たせなくなります。（つまり、変数がスコープを抜けるときに手動で解放処理しないといけなくなる）

CMemory(派生クラスを含む)を廃止したい、であれば賛成できる余地がありますが、そのためには準備作業が必要だと思っていて、現状では賛同できません。

「劣化版スマートポインタとしてのCMemory」ってのがよくわかりません。コード例で示してもらう事は出来ますか？

一般的にデストラクタを virtual にするのは派生クラスを動的に確保したものを後で解放する際に 基底クラスのポインタ経由で行うと派生クラスのデストラクタが呼ばれないのが問題になります。
でもそういうところってありますか？

あと現時点では派生クラスの CNative にも CNativeW にもデストラクタ追加していないし基本的にメンバー追加もしていない事から問題無いのではないでしょうか？

これはぼくの勘違いかもしれません。
CMemoryに仮想デストラクタがないとCNativeWのインスタンスを削除するときにCNativeWのデストラクタ(コンパイラにより自動生成される)が呼ばれて終わりだと思っています。あとで検証してみます。（ＴＯＤＯ：やってみる。

CNativeWのデバッグポインタは、natvisを使って代替できます。
前にビルドエラーの原因になったときに、現代的手法への差し替えを提案したことがあります。

このPRの対応だと単にデバッグ変数を削ってるだけなのであんまり良くない気がします。

.natvis は使いたければ各自で整備してもらえば良い事だと思います。

「不要だ」と判断した場合を除いて、既存機能を削るには代替を提供すべきだと思います。
文脈的に、不要と判断したわけではないと思うので、削るならpdb埋込の.natvisを提供したいっす。

[beru]

ただ、変える必要があるのかないのか？は気にしたいと思っています。

• 必要なければ変えないがいいです。
• 必要だから変えるにしたいです。

容量を節約する為に変えています。あれ？話が最初に戻ってるよな…。

容量を節約するために型に手を入れようとしているメンバが「すべてpublicな件」も関係してそうな気がします。ぼく個人は、得意な言語がほぼOOPなこともあり、基本的にオブジェクト思考です。全メンバがpublicだと設計をサボったうん○クラスに見えてしまいます。仮にメンバがカプセル化されていたなら、内部実装をintに変更してビットフィールドにしても影響は出ないわけで😄

C++言語の機能の活用はしたいですね。

え、そうなの？
char, struct {int}, charの並びで整列させると
char, padding, struct {int}, char になりそうな気がしますが・・・（←試してない

拡張情報構造体のサイズを7バイトにまで節約したんですが、その後にパディング無しで EOL のメンバーを入れる事で CDocLine 全体のサイズを 40 にまで節約する事が出来ました。

表とかで解説用意すれば分かりやすいと思いますが、どこかに型レイアウトの表を自動生成するコードが無いかなぁ…（さぼり。

これはぼくの勘違いかもしれません。
CMemoryに仮想デストラクタがないとCNativeWのインスタンスを削除するときにCNativeWのデストラクタ(コンパイラにより自動生成される)が呼ばれて終わりだと思っています。あとで検証してみます。（ＴＯＤＯ：やってみる。

コンパイラが派生クラスのインスタンスだと判断出来るケースでは基底クラスのデストラクタの呼び出しもちゃんと自動的に追加されますよ。そうじゃないとプログラミング言語として使い勝手が悪すぎるでしょう。

「不要だ」と判断した場合を除いて、既存機能を削るには代替を提供すべきだと思います。
文脈的に、不要と判断したわけではないと思うので、削るならpdb埋込の.natvisを提供したいっす。

ウォッチウィンドウで書式指定すれば wchar_t* として見れるだろうから不要と判断しました。あとデバッグ時に CDocLine のサイズが増えてしまうのを避ける為でもあります。あれ？話が最初に戻ってるような…。

[berryzplus]

ただ、変える必要があるのかないのか？は気にしたいと思っています。
•必要なければ変えないがいいです。
•必要だから変えるにしたいです。

容量を節約する為に変えています。あれ？話が最初に戻ってるよな…。

内部実装をintに変えたらビットフィールドにできますよね？
つまり、容量を節約するために、他の手段がないわけではない・・・
（まーぶっちゃけ、アクセサ用意するのが面倒だってのは分かりますけど）

え、そうなの？
char, struct {int}, charの並びで整列させると
char, padding, struct {int}, char になりそうな気がしますが・・・（←試してない

拡張情報構造体のサイズを7バイトにまで節約したんですが、その後にパディング無しで EOL のメンバーを入れる事で CDocLine 全体のサイズを 40 にまで節約する事が出来ました。

理解。7bytesのstructの直後1バイトが埋まったっちゅーことですね。

例示はstructの開始アドレスが奇数な場合はpaddingされるんじゃないか？ということを言ってました・・・あとでやってみます:smile:

「不要だ」と判断した場合を除いて、既存機能を削るには代替を提供すべきだと思います。
文脈的に、不要と判断したわけではないと思うので、削るならpdb埋込の.natvisを提供したいっす。

ウォッチウィンドウで書式指定すれば wchar_t* として見れるだろうから不要と判断しました。あとデバッグ時に CDocLine のサイズが増えてしまうのを避ける為でもあります。あれ？話が最初に戻ってるような…。

すんません、言葉が足らんかったです。
ちょっと設定すれば見れるから大丈夫、の見解に対して、それじゃ足らん、と主観を述べております。
変更前は書式指定しなくても見れてたので、そうなるように対処したいです。

ここだけ切り出してこちらでPR作成してもいいですが、どうします？
（「それめんどい」という反応は想定内でした）

[berryzplus]

「不要だ」と判断した場合を除いて、既存機能を削るには代替を提供すべきだと思います。
文脈的に、不要と判断したわけではないと思うので、削るならpdb埋込の.natvisを提供したいっす。

ウォッチウィンドウで書式指定すれば wchar_t* として見れるだろうから不要と判断しました。あとデバッグ時に CDocLine のサイズが増えてしまうのを避ける為でもあります。あれ？話が最初に戻ってるような…。

ぼくが読み違ってますね。

「不要と判断したわけではないと思う（主観）」に対して、
「不要と判断しました（主観）」と言ってる。

主たる目的がサイズ削減で、不要と判断したゴミを削るだけ。
「ゴミの代替なんて当然用意しません」で理屈は合う気がします。

そうなると、ここの指摘は「本当にただのゴミ？」ってとこに争点が移ります。
もちろん、ぼくは「ゴミってほど不要な情報でもなくね？」と思っています。

[beru]

内部実装をintに変えたらビットフィールドにできますよね？
つまり、容量を節約するために、他の手段がないわけではない・・・
（まーぶっちゃけ、アクセサ用意するのが面倒だってのは分かりますけど）

コードの差分をあまり増やさない形でいけるなら良いですね。

CLineModified, CLineDiffed, CLineBookmarked, CLineFuncList 型のメンバーを拡張情報用の構造体にそのまま持たずにゲッターセッター用意すれば行けると思いますが、差分が増えるので（ ≒ 大変なので）行いませんでした。

例示はstructの開始アドレスが奇数な場合はpaddingされるんじゃないか？ということを言ってました・・・あとでやってみます😄

offsetof(CDocLine, m_sMark) は 24 32 なので奇数にはならないと思います。

すんません、言葉が足らんかったです。
ちょっと設定すれば見れるから大丈夫、の見解に対して、それじゃ足らん、と主観を述べております。
変更前は書式指定しなくても見れてたので、そうなるように対処したいです。

ここだけ切り出してこちらでPR作成してもいいですが、どうします？
（「それめんどい」という反応は想定内でした）

https://devblogs.microsoft.com/cppblog/project-support-for-natvis/

プロジェクト単位の natvis がサポートされていたのを知りませんでした。

自分がそのデバッグ用のメンバーを使っていない事もあって有難味が分かっていないのかもしれないですね。ただ構造体のサイズを増やさない方法で同等の事が実現出来るのであればそれが良いと思います。

@berryzplus さんの方で先にデバッグ用メンバー削って natvis ファイルをプロジェクトに追加する PR を用意してもらっても良いでしょうか？

[beru]

「不要と判断したわけではないと思う（主観）」に対して、
「不要と判断しました（主観）」と言ってる。

主たる目的がサイズ削減で、不要と判断したゴミを削るだけ。
「ゴミの代替なんて当然用意しません」で理屈は合う気がします。

そうなると、ここの指摘は「本当にただのゴミ？」ってとこに争点が移ります。
もちろん、ぼくは「ゴミってほど不要な情報でもなくね？」と思っています。

自分が必要としてる派の心の声を受信出来ていないんだと思います。

Releaseビルドではプリプロセッサで消えるのでそこまで存在を消さないと困るという訳では無いです。というか消したのって基底クラスのデストラクタの virtual を外しても問題が起きないようにそうしています。派生クラスで追加メンバーを入れない前提を確実にしてポリモーフィズムなんていう富豪的な発想が CMemory から派生する型に持ち込まれる事が無いようにしたかったんです。

まぁとはいえ普段編集するファイルって多くても数千行くらいなのでそこまでサイズ切り詰めるためにケチケチする必要は nothing と言われたら言い返せません。8バイト削れたら 2^24行で 128MB節約できるのはおいしいですけど、その考えをエスカレートさせるとポインタの代わりに index を使うように大改造を進めてしまいそうです。。

[berryzplus]

@berryzplus さんの方で先にデバッグ用メンバー削って natvis ファイルをプロジェクトに追加する PR を用意してもらっても良いでしょうか？

うぃ。準備中っす。
appveyor待ちなう 😄

デバッガ使って調べてみたんですけど、変更前のCDocLineのサイズはこんなんでした。

ビルド環境	sizeof(CDocLine)	備考
x64 - Debug	64
x64 - Release	56	PR本文の56はコレ。
Win32 - Debug	44
Win32 - Release	40

[beru]

なおこの PR では sizeof(CDocLine) を 56 から 40 に削減していますが、ビットフィールド等を使って拡張情報構造体のサイズをこれ以上縮めたとしても x64 ビルドでは CDocLine のアライメントが 8 バイトなので実質的に意味が無い気がします。

拡張情報構造体の位置が CDocLine 先頭から 32 なので（offsetof(CDocLine, m_sMark) は 32 ）…。

型	メンバー名	offsetof	sizeof
CDocLine*	m_pPrev	0	8
CDocLine*	m_pNext	8	8
CNativeW	m_cLine	16	16
MarkType	m_sMark	32	7
CEol	m_cEol	39	1

[AppVeyorBot]

✅ Build sakura 1.0.2147 completed (commit 38dc64fab9 by @)

[berryzplus]

これはぼくの勘違いかもしれません。
CMemoryに仮想デストラクタがないとCNativeWのインスタンスを削除するときにCNativeWのデストラクタ(コンパイラにより自動生成される)が呼ばれて終わりだと思っています。あとで検証してみます。（ＴＯＤＯ：やってみる。

コンパイラが派生クラスのインスタンスだと判断出来るケースでは基底クラスのデストラクタの呼び出しもちゃんと自動的に追加されますよ。そうじゃないとプログラミング言語として使い勝手が悪すぎるでしょう。

確認完了、ぼくの勘違いでした。
PRをcheckoutした状態で、以下のtestコードを実行し、CMemoryのデストラクタがちゃんと呼ばれるのを確認できました。

	{
		CNativeW test(L"this is test");
	} // test を定義したスコープがここで終わるから、オブジェクトは破棄される

奇数にはならないと思います。

ならNP(No Problem)っちゅーことで。

なおこの PR では sizeof(CDocLine) を 56 から 40 に削減していますが、ビットフィールド等を使って拡張情報構造体のサイズをこれ以上縮めたとしても x64 ビルドでは CDocLine のアライメントが 8 バイトなので実質的に意味が無い気がします。

x64にはなんか制約があったんですよね、確か。　＞　アラインメントが8バイトなので

確保が2の累乗単位なので 56 ⇒ 40 にする意義が微妙な気もしますが、memory_resourceの本来の用途は、様々な型でメモリ確保・開放を共有できる機構を提供することにあるっぽいので、サイズ削減により64との差が開くことには意味があると思っています。

[berryzplus]

疑問はすべて解消したので、進めて良さそうに思います。
対応ありがとうございます。

[beru]

なおこの PR では sizeof(CDocLine) を 56 から 40 に削減していますが、ビットフィールド等を使って拡張情報構造体のサイズをこれ以上縮めたとしても x64 ビルドでは CDocLine のアライメントが 8 バイトなので実質的に意味が無い気がします。

x64にはなんか制約があったんですよね、確か。　＞　アラインメントが8バイトなので

x64 だとポインタが 8バイトなので、ポインタ変数のアライメントは 8 バイトになるようにコンパイラが自動的に調整するんじゃないかと思います。#pragma pack で詰めたりした場合にどうなるかは未確認ですが。。なんでアライメントを調整するのかはパフォーマンスが低下しないようにするという点と、あとプロセッサによっては奇数アドレスアクセスでバスエラー例外（割込み）が出たりしますね。

確保が2の累乗単位なので 56 ⇒ 40 にする意義が微妙な気もしますが、memory_resourceの本来の用途は、様々な型でメモリ確保・開放を共有できる機構を提供することにあるっぽいので、サイズ削減により64との差が開くことには意味があると思っています。

確保が2の累乗単位なのはVC++の std::pmr::unsynchronized_pool_resource の実装でした。CPoolResource.h の実装ではそんな事は無いので CDocLine のサイズを削減した分だけメモリ使用量も削減されます（ただしアライメントの分の隙間が入ることは有ります）。

[beru]

@berryzplus さん

レビューありがとうございます。マージします。
この変更による問題が見つかった場合は別PRで対処します。