行データ管理クラスでメモリプールを使う事でメモリ確保と解放処理を高速化 #980
Conversation
|
❌ Build sakura 1.0.2094 failed (commit f90149d767 by @) |
| #include <memory> | ||
| #include "util/design_template.h" | ||
|
|
||
| template <typename T, size_t BlockSize = 4096> |
There was a problem hiding this comment.
追加しました。クラスのI/Fを意図的にミニマムにしたので試験内容も単純なものになっています。
|
❌ Build sakura 1.0.2097 failed (commit 0b5d19fc76 by @beru) |
|
❌ Build sakura 1.0.2098 failed (commit 07882dd744 by @beru) |
|
❌ Build sakura 1.0.2099 failed (commit b24caab739 by @beru) |
|
❌ Build sakura 1.0.2100 failed (commit 2d2da1145a by @beru) |
|
✅ Build sakura 1.0.2101 completed (commit 1e024d0471 by @beru) |
| } | ||
|
|
||
| private: | ||
| union Node { |
There was a problem hiding this comment.
下記の2つの理由が考えられます。
- 領域を再利用する為に要素型と自己参照用のポインタを同じ領域に割り当てる
- 共用体のサイズは各メンバを格納できるサイズになる事を利用
There was a problem hiding this comment.
自己参照用のポインタと要素型を同じ領域に配置して問題ないのですか?
可能であれば、アスキーアート(あるいは SVG とか)でデータ構造の説明が欲しいです。
There was a problem hiding this comment.
134行目の
body = std::align(alignof(Node), sizeof(Node), body, space); からすると
こんな感じ?
+----------------------------------+
| V
+-----------+---------+------------+-----------+---------+------------+
| ポインタ | | | ポインタ | | |
| next | padding | T のデータ | next | padding | T のデータ |
| | | | | | |
+-----------+---------+------------+-----------+---------+------------+
^ | ^
| +---------------------------------+
タイプ T のサイズ向けに align
There was a problem hiding this comment.
自己参照用のポインタは要素が破棄されて未割当領域になった際に使われるので問題無い認識です。ブロック先頭部分のポインタはブロック間を接続するのに使われて、先頭部分に要素は記録されない為問題無いです。
ブロックを構造体型として定義した方がコードの見通しが良くなるかもしれないですね。
There was a problem hiding this comment.
@m-tmatma さん
可能であれば、アスキーアート(あるいは SVG とか)でデータ構造の説明が欲しいです。
図をPlantUMLとかで用意する事は出来ますが、行動に移すのを躊躇する何かが有ります。
今回作成したメモリプールの実装は100行未満の低機能な単純なものなので、分かりやすいコードの書き方にする事に注力した方が良いかも知れません。
There was a problem hiding this comment.
別に図にしなくても構造を説明する文章でもいいですが
第三者が理解できる説明でもいいです。
There was a problem hiding this comment.
現状のコード読んで作りが理解できないですか？それならこのPR自体諦めますけど。
There was a problem hiding this comment.
コメントを書いてほしい理由は、初めて見た人が
パッと構造を理解できる助けになるようにして
ほしいからです
| #include "util/design_template.h" | ||
|
|
||
| template <typename T, size_t BlockSize = 4096> | ||
| class CMemPool final |
There was a problem hiding this comment.
データ構造やロジックの説明をコメントに追加してほしいです。
内部仕様と、外部仕様も。
sakura_core/mem/CMemPool.h
Outdated
| Node* next; | ||
| }; | ||
|
|
||
| static_assert(BlockSize >= 2 * sizeof(Node), "BlockSize too small."); |
There was a problem hiding this comment.
ブロック領域のサイズがNode2個分以上でなければいけない、という確認です。
何故 2 なのか？という疑問が湧きますが、あんまり厳密なチェックではないと思います。
- 要素型のサイズがポインタのサイズ以下のケース（Nodeのサイズはポインタのサイズ以上）
- ブロック領域の先頭にはポインタが来ます。残りの領域のサイズも最低ポインタ分が無いと解放時に未割当領域に出来ません。ブロック領域のサイズがNode2個分以上ならそれを満たします。
- 要素型のサイズがポインタのサイズより大きいケース（Nodeのサイズは要素型のサイズ以上）
- ブロック領域の先頭にはポインタが来ます。残りの領域のサイズは Node 1つ以上有れば良いですが、ブロック領域のサイズがNode2個分以上なら条件満たします。
ケースを限定すればもっと必要なサイズを絞れる気もしますが、まぁ気張らないで今のやり方でも良いとは思います。
There was a problem hiding this comment.
コメントには Node2個分以上 にする理由を記載いただきたいです。
Node2個分以上というのは見たらわかるので。
sakura_core/mem/CMemPool.h
Outdated
| return ret; | ||
| } | ||
| else { | ||
| Node* border = reinterpret_cast<Node*>(reinterpret_cast<char*>(m_currentBlock) + BlockSize - sizeof(Node) + 1); |
There was a problem hiding this comment.
ブロック領域中の残りの部分の大きさが Node の大きさ未満だともうそのブロック領域からは Node を切り出す事が出来ません。それを判定する際に使うポインタ変数のアドレス計算です。
There was a problem hiding this comment.
データ構造はまだ理解できていませんが、
このコードを見たときに、 99 行目で +1 しているのは 100 行目で >= にしているからで
100 行目で > にすれば 99 行目の +1 はいらないんじゃないかと思ってコメントしました。
There was a problem hiding this comment.
判定的に等価になるならどっちでも良いかと思います。
There was a problem hiding this comment.
細かいこと言うと、+1 の演算が余計に入るので現状コードのほうが少し遅い。
There was a problem hiding this comment.
もうちょっとロジックを理解してから再度コメントするか考えます。
tests/unittests/test-cmempool.cpp
Outdated
| } | ||
|
|
||
| // ブロックサイズ | ||
| TEST(CMemPool, BlockSize) |
There was a problem hiding this comment.
もちろんブロックサイズのテストをしようとしているのはわかります。
There was a problem hiding this comment.
#980 (comment) について確認するテストです。
There was a problem hiding this comment.
個別のケースに関して具体的にどのような観点でテストしようとしているのかの
コメントがないと今後のメンテ等難しくなるので、ロジックや期待動作の
説明が欲しいです。
所感ぼんやりと眺めてる限り、いい感じだと思います。 保留としたい理由
ちなみにデータ管理系クラスとエディタとの関連はこんな感じ。 ・
変更内容をキチンと理解できるまでは保留としたいです。 どうなったらＯＫと言えるか？PR全文を頭から読み返してみました。 目的「ログファイル等の行数が多いファイルを読んで表示するのに掛かる時間を減らす。」 まず、目的と手段があってるかについて自信がないっす。 「巨大ファイルを開くのに時間がかかるのは、メモリ確保の速度が遅いからだ」という検証結果が欲しいです。常にそうでなくて構わないのです。ファイルオープンが遅い理由は他にもたくさんあるので、他の理由との兼ね合いで速度が出ない状況もありえると思います。 特定の状況で、メモリ確保が遅いことによりファイルオープンが遅くなる、が証明できれば細かいことは気にしなくていいのかな、と思っています。 あ、単体テストはぼくも欲しいです 😄 単体テストがあると、追加されるクラスが何を意図して作られたものかが明確になるからです。 |
|
ああ、失礼。
|
|
❌ Build sakura 1.0.2102 failed (commit e0c384fbe5 by @) |
要素ごとに分析して個々に処理時間測定を行う事はしていません。 メモリプールを使う事で行数がとても多いファイルを開く処理の時間が短くなる（メモリ使用量も減る）事は、そういうファイルを開く操作を行えば知覚出来るぐらいなので、あえてプログラムを改造したりしてベンチマークを取らないでも良いかなと思ってます。 まぁ結構適当なスタンスではあります。。問題が無い事を自分で色々な角度でしっかりと確認して検証すべきなのはそうなんですが、ヘタレなんでお手柔らかにお願いします。 |
|
✅ Build sakura 1.0.2103 completed (commit e0ad569349 by @) |
|
✅ Build sakura 1.0.2105 completed (commit 7689d57a9f by @) |
|
@berryzplus さん
|
| class CMemPoolTest : public ::testing::Test{}; | ||
|
|
||
| using test_types = ::testing::Types< | ||
| std::integral_constant<std::size_t, 512>, |
|
✅ Build sakura 1.0.2107 completed (commit b7619f4534 by @beru) |
tests/unittests/test-cmempool.cpp
Outdated
| // ブロックサイズは要素が２つ以上入る大きさにする確認 | ||
| TEST(CMemPool, BlockSize) | ||
| { | ||
| CMemPool<std::array<uint8_t, 1024>, 2048> pool0; |
There was a problem hiding this comment.
クラスをクラスタンス化するだけだと、AllocateBlock が呼ばれるだけですが、
Construct や Destruct を呼ぶ必要はないですか?
There was a problem hiding this comment.
色々な要素型の場合にBlockSizeを最小限の大きさにした場合に正常にCMemPoolが動作するかの確認をした方が良さそうですね。
|
✅ Build sakura 1.0.2110 completed (commit e695e2c0f1 by @beru) |
|
✅ Build sakura 1.0.2113 completed (commit 3ab2590765 by @beru) |
|
データ構造は という構造でしょうか？ それなら |
そうです。パディングは常にあるわけではなく T のデータの配列のアライメントを取るのに必要な場合にのみ存在します。 8110e42 で構造体を使うように書き方を変更しましたが、構造体の各メンバーのアライメントは自動で取られるので、ポインタ操作の記述が省けます。
その考えには同意できないです。コードを提示してもらえば @m-tmatma さんが言うシンプルさというのがどんなものなのかを検討してみますが。 |
| ~Block() {} | ||
| struct { | ||
| Block* next; | ||
| Node nodes[1]; |
There was a problem hiding this comment.
もともとのコードではここに padding 入れてませんでしたか？
コンパイラにお任せですか？
There was a problem hiding this comment.
はい。もともとのコードでは、CMemPool::AllocateBlock メソッドで std::align 関数を使ってアライメントを取っていましたが、構造体を使う事でコンパイラに任せるようにしました。
There was a problem hiding this comment.
static_assert 等などで、それを保証した方がいいと思います
There was a problem hiding this comment.
変数のアドレスは動作時に決まるものなので、静的には保証出来ないのではないかと思い当たりました。
なので行うべきは CMemPool::AllocateBlock プライベートメソッドにおいて operator new で確保した領域を Block* にキャストして m_currentBlock メンバーに割り当てる際に、std::align を使って確実にアライメントが取られるようにする事、な気がします。
単体テストでももっと大きいアライメントを要求する型を用意して確認する事にします。
| union Node { | ||
| ~Node() {} | ||
| T element; // 要素型 | ||
| Node* next; // 解放後の未割当領域の場合は次の未割当領域に繋がる |
There was a problem hiding this comment.
element が有効か、next が有効かはどんなロジックで
判断されますか？
There was a problem hiding this comment.
- element として扱う場合（next としては扱わない）
CMemPool::Constructメソッドが呼び出されるとそこからCMemPool::Allocateプライベートメソッドを呼び出して割り当てる領域(Node)の要素(Element)のアドレスを返します。それ以降は破棄されるまではその領域は要素として扱われます。
- next として扱う場合（element としては扱わない）
CMemPool::Destructメソッドが呼び出されると要素のデストラクタ呼び出しを行って破棄し、Nodeを未割当の領域として扱うようになります。要素として扱わなくなった時点で領域 を自己参照型として扱えるようになります。
There was a problem hiding this comment.
観点が逆です。
element として扱う場合とnext として扱う場合の
場合分けをする判断基準が知りたいです。
心配してるのは、element のデータを next とみなしたり
その逆になるパターンがないか？です
There was a problem hiding this comment.
もしそういうパターンに陥るケースがある事に気づいたりコードにロジック上の不備を見つけたのならそれを指摘して下さい。延々と体裁とかに関する要求をされて不毛な感じです。
メモリプールクラスを汎用的にして品質を高める事に注目したいわけではなくて、パフォーマンスを改善したかったんですが…。
sakura-editor/management-forum#8
を再度読んでください。
There was a problem hiding this comment.
コメントを書いてほしいというのは
別に体裁に関するコメントではありません。
@beru さんの考えているより、このクラスは
簡単ではないと思います。
最初にデータ構造をしめす説明をするなり
コメントなりがあるとレビューする側の時間を
節約できます。
PRを作成する人は、多くの時間を割いていますが、
同時に レビューする側も同様に時間をかけています。
レビューする側の時間を節約するという観点も
持って頂けると有難いです。
There was a problem hiding this comment.
コメントで説明をごてごてに書く事よりコードの記述に注力しないと結局出来上がるものに寄与しません。
データ構造の説明って最初からメモリプールって書いてるじゃないですか…。コード読んで分からなかったら頭の中でシミュレートしたり実際にビルドして動かしてロジックの内容をつかんでからレビューコメント書くべきですよ。
There was a problem hiding this comment.
コメントで説明をごてごてに書く事よりコードの記述に注力しないと結局出来上がるものに寄与しません。
まず認識していだだきたいのは、レビューに時間を割いているのは
PR に期待しているからです。
大して価値のない PR ならレビューに時間はかけないです。
価値のあるものだから、よりよくしようと意見を言うわけです。
コメントというのは単に手段であって目的ではないです。
コメントという形をとっていますが、どちらかというと
設計および仕様を残してほしいという意図です。
この PR がマージされたあと、コードを書いた人とは別の人が容易に
変更できることを目的にしてコメントを書いてほしいといっています。
データ構造の説明って最初からメモリプールって書いてるじゃないですか…。
メモリプールといってもどういう構造をとるかはいろいろあると思います。
コード読んで分からなかったら頭の中でシミュレートしたり実際にビルドして動かしてロジックの内容をつかんでからレビューコメント書くべきですよ。
コメントを書くのは嫌なのは想像しますが、
レビューアーがレビューしやすいような PR を作成するのは重要ですよ。
そうすることで早くマージされる可能性が上がるとおもいます。
メモリプールクラスを汎用的にして品質を高める事に注目したいわけではなくて、パフォーマンスを改善したかったんですが…。
パフォーマンスを改善するために、バグを生んでは元も子もないので、
基本的な部分で使われるクラスに関しては、バグのないようにするのか
基本かと思います。
There was a problem hiding this comment.
観点が逆です。
element として扱う場合とnext として扱う場合の
場合分けをする判断基準が知りたいです。
まず当たり前な前提として、element として扱うのか、それとも next として扱うか、というフラグ的な記録は有りません（用意する実装方法も考えられますが）。にもかかわらずライフサイクルにおいて element として扱われるか、それとも next として扱われるかが変化しています。メモリを参照するロジックの記述場所によって扱いが別れているわけです。
CMemPool クラス側のメソッド内の記述では、どちらとして扱っているのかはコード上に現れています。ただどうしてそういう記述にしているのか？が不明瞭なのかもしれません。
未割当領域と呼んでいるのは、CMemPool::Destruct の呼び出しが行われた領域ですが、それらは next として扱います。どうして next として扱えるのか？についてですが、解放して element としての役割をいったん終えた為にその領域を next として扱えるからです。「扱えるからと言ってどうして扱うのか？」ですが未割当になったNodeを next メンバーを使って連結して後で要素構築時にその未割当領域から確保するようにしてメモリの再利用をするからです。再利用をやっておかないと、確保と破棄を繰り返すうちにメモリ使用量がどんどん増えてしまうのでこれは必要な事です。
次に element として扱う場合ですが、CMemPool::Construct の呼び出しが行われてそこで CMemPool::Allocate の呼び出しを行って返却された領域を element として扱います。「どうしてそれらを element として扱うのか？」というのはなんだか微妙な問いに思えますが、まず確保したブロック領域の中を切り出して element として利用しないとメモリプールとして成り立たないのでそうします。では問いを変えて「どうしてそれらの領域は element として利用出来るのか？」ですが element として利用する間は next は扱わないライフサイクルに入るからです。一つ前の段落とは裏返しですね。
さて、上記の説明ですがそんなん答えになっていないぞと言われるとちょっと禅問答なコースに入りそうですが…、まぁもしそう思われるとしたら私の説明が至らないせいでしょうか…。
心配してるのは、element のデータを next とみなしたり
その逆になるパターンがないか？です
素朴ですが良い質問だと思います。
そういう事が絶対に起きえない、とは言えないと思います。どういう時に起き得るかというのを考えてみると、
CMemPoolクラスの利用者側（直接的にはCDocLineMgrクラス）がポインタの扱いを不適切にして、本来扱うべきではない領域までメモリの書き込みをしてしまう場合- これについては生ポインタがどしどし使われているので、気を付けて下さい、としか言えません。
CMemPool::Destructに不正なポインタが渡されるケース- 不正なポインタというのは、
CMemPool::Constructメソッドが返したポインタではないアドレスのポインタや、また以前にCMemPool::Destructを呼び出して破棄済みなのに再度続けてCMemPool::Destructを呼び出す等した場合 - そもそも
CMemPool利用者側が変なコードを書くべきではありませんが、CMemPool側で取れる対策としては自分が確保したブロック領域群のアドレスの範囲内かどうかを確認するコードをデバッグビルドの時にチェック用に入れる、等が考えられます。
- 不正なポインタというのは、
CMemPool::Destructを呼び出した後にその領域は既に破棄済み扱いなのに、CMemPool利用者側が element として扱ってしまう場合- これについても
CMemPool利用者側が変なコードを書くべきではありませんが、CMemPool側で取れる対策としては、デバッグビルドでは0xcdcdcdcd等のパターンで next ポインタ以外の領域を初期化するコードをCMemPool::Freeに入れるとかでしょうか？
- これについても
さてやたらと回答が長文になってしまいましたが、文章力が貧弱なせいかもしれません。とほほー。
There was a problem hiding this comment.
まず認識していだだきたいのは、レビューに時間を割いているのは
PR に期待しているからです。大して価値のない PR ならレビューに時間はかけないです。
価値のあるものだから、よりよくしようと意見を言うわけです。
ありがとうございます。
コメントというのは単に手段であって目的ではないです。
コメントという形をとっていますが、どちらかというと
設計および仕様を残してほしいという意図です。
ソースコードも設計および仕様の一形態ですが、C/C++は形式言語ではないですし静的解析も行いにくいのでそれだけでは限界があるかもしれないですね。
@m-tmatma さんの要求って、レビューアーが細部を理解出来る事は当然として、バックグラウンドが様々な第三者が見ても内容がすっきりと分かるくらいの説明を残してほしいという事ですよね？それは理想的かもしれないですけど、そこまで本当に必要なんでしょうか？
分かる人には分かる、的なコードばっかりだとメンテが確かにきつくなるとは思いますけど、ドキュメント文化におんぶにだっこしたがりマンばっかりだとそれもなぁ…。
この PR がマージされたあと、コードを書いた人とは別の人が容易に
変更できることを目的にしてコメントを書いてほしいといっています。
なんだかその調子で行くと、拡張性が足りない、汎用性が無い、とか色々な要求も上乗せされそうですね…。
メモリプールといってもどういう構造をとるかはいろいろあると思います。
そりゃそうでしょうけど、教材を作っているんじゃないんだし懇切丁寧なコメントとテストとドキュメントを揃えないと取り込めない、ってなるとキッツいです。
コメントを書くのは嫌なのは想像しますが、
レビューアーがレビューしやすいような PR を作成するのは重要ですよ。
そうすることで早くマージされる可能性が上がるとおもいます。
そうですねぇ。レビューアーは審査官みたいなものですね。
パフォーマンスを改善するために、バグを生んでは元も子もないので、
基本的な部分で使われるクラスに関しては、バグのないようにするのか
基本かと思います。
私も他力本願ですが、レビューアーがコメントが無いコードを読解してバグを指摘してくれたら楽だなぁ、と思います。
|
テンプレートを利用したメタプログラミングの技法を使えば、メモリプール内の割り当て済み領域と未割当領域を「型」で区別できるようになります。ただ、それをやるとこれほどシンプルな修正にはならんです。 virtual_allocator(メモリプール機能付きメモリアロケータ) とか作ってみたことあるんですが、適用するための修正箇所が膨大過ぎて PR する気にもならんかったです。出すとたぶん「boostで良くね？」とか、結論を出しづらいメンドクサイ展開になりそうなのでまだ当分保留するつもりです 😢 メモリプールの実装概念自体それなりに難解なので、「CMemPoolさえ理解できればなんとかなる」ようにまとめたこの修正の価値は大きいと思います。 このPRは閉じてしまってるので一旦終了なんですけど、今いるメンバーで C/C++ に一番詳しいのはたぶん @beru さんなので、また力を貸していただけると幸いに思っとります 😄 |
|
@berryzplus さん
C++20 になればプリミティブなライブラリを用意せずとも標準ライブラリのもので間に合うケースが増えそうですね、自作が必要なケースは決して無くなりはしないでしょうけれど。 レガシーコードの書き換え自体も大変だと思いますが、テストも合わせて用意しないと挙動を壊してしまった事に気づきにくいので容易な道では無いと思います。 言われてみると allocator クラスを作るのが普通のC++ですね、このPRではサクラエディタの他のクラスに足並みを揃えたクラス名にして、あと利用ケースが限定されているので余計なI/Fは付けずに実装もミニマムにしてみました。しかしOOPの体面を取り繕おうとして
自分が一番詳しいとかせもまつですね。きっと将来こんな感じですね。 |
|
このPRによる修正でファイル読込が速くなる理由について解説しときます。
PRで改善されるのは、ファイル内の行数が極端に多い場合の読込速度です。 なぜか？ CDocLineのためのメモリ確保を小さなサイズで個別に行っていたのを、大きなサイズでまとめて行うように変えたからです。ヒープマネージャーに対する問い合わせの回数が減った分だけ速度が改善したものと考えられます。 メモリ要求 （param: サイズ） ヒープのメモリ確保は実際にはヒープサイズ単位で行われる。通常4Kごと。 ページサイズを超えない範囲でヒープサイズの整数倍となるようなサイズを指定すれば効率よくメモリ確保できるはず。 |
|
@berryzplus さん 細かい点ですが自分の理解と食い違っている点をコメントします。 ページサイズは通常 4KiB だそうです。 SYSTEM_INFO si;
GetSystemInfo(&si);
assert(si.dwPageSize == 4096);
assert(si.dwAllocationGranularity == 1024*64);
for (size_t i=0; i<=1024*64*2; ++i) {
LPVOID p = VirtualAlloc(NULL, i, MEM_COMMIT | MEM_RESERVE, PAGE_READWRITE);
assert(((ptrdiff_t)p % (1024*64)) == 0);
VirtualFree(p, 0, MEM_RELEASE);
}なんでページサイズが4Kなのに仮想アドレスが64KでアライメントされているのかについてはMSの有名な開発者 Raymond Chen さんのブログに書かれていて、Alpha AXPプロセッサの命令セットでも効率よく動かすためにそうしたみたいです。 一般的なヒープマネージャーの処理内容については自分で1から実装出来るほどきちんと詳細には把握していませんが色々なメモリ確保と解放の使用状況に対応する為に結構様々な処理をやっていると思います。自前のメモリプールでは用途を限定して単純な事しか行っていないのでその分だけ処理時間が短くなるのだと思います。 |
ウソ書きました。 タバコの発注単位に置き換えると、1本とか1箱(=20本)じゃなくて、 メモリ確保単位はシステムによって違うことになっているので、const切らずに実行時にGetSystemInfoするのが定石っぽいです。新しいPRでメモリ確保単位が constexpr になってるのを見て「ぬ？」と思ったことはまだ秘密です。 |
CRTのヒープマネージャーやOSヒープは汎用的に作られていて比較的遅いのでそれらを叩く回数を減らす事で速くする、という点は上記の例えでイメージは伝わるかもしれないですね。 付け加えていうと、まとめて確保したメモリの管理を簡易的なメモリプールが受け持ちますが、それがやる仕事内容は単純な為にすぐに応答を返せるのでたくさん呼び出しが行われても比較的早く片付きます。 無理やり例えていうと遠くにある喫茶店やレストランとは違って、自販機や給水機は単純だけど速い、とかでしょうか？
ハードウェアアーキテクチャが異なると VirtualAlloc による最小メモリ確保単位が 64K じゃない事もあるのかもしれないですね。x86/x86-64 では 64K なので決め打ちでも良い気もしますけれど…。 新しいPRでメモリ確保単位をテンプレート引数で指定するのではなくて 64K 固定にしたのは、指定出来ないと困る用途は今のところ無いと判断したからです。 |
PR の目的
ログファイル等の行数が多いファイルを読んで表示するのに掛かる時間を減らす。
カテゴリ
PR のメリット
（行数が）極端に大きいファイルを開く時の待ち時間が短くなります。
あと分かりづらいですがそのファイルを閉じる時の時間も短くなります。
追記：テストケースとしてちょっと極端な例ですが、a\r\n という行が 16777216行連続しているファイルを開いてから編集出来るようになるまでの待ち時間が短くなるのが体感できます。
そのファイルをzipで圧縮したもの → test.zip
PR のデメリット (トレードオフとかあれば)
メモリ使用量が思ったより減らないといった事態が考えられます。
メモリプールのメモリ解放のタイミングが行データの管理クラス（
CDocLineMgr）が解放されるタイミングなので、エディタのプロセスを閉じるまでメモリプールのメモリ解放がされない事になります。ただそれで困るという事はあんまり考えられない気もします。
PR の影響範囲
行データ管理クラス（
CDocLineMgr）関連関連チケット
#312
過去に #312 で同様の事を行いましたが、外部プロジェクトのソースコードを使っていたので取り込むのに障害が有りました。今回は同様の処理を自前で書いているのでその問題は無いです。