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

行データ管理クラスでメモリプールを使う事でメモリ確保と解放処理を高速化 #980

Closed
wants to merge 10 commits into from
1 change: 1 addition & 0 deletions sakura/sakura.vcxproj
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,7 @@
<ClInclude Include="..\sakura_core\macro\CWSHManager.h" />
<ClInclude Include="..\sakura_core\mem\CMemory.h" />
<ClInclude Include="..\sakura_core\mem\CMemoryIterator.h" />
<ClInclude Include="..\sakura_core\mem\CMemPool.h" />
<ClInclude Include="..\sakura_core\mem\CNative.h" />
<ClInclude Include="..\sakura_core\mem\CNativeA.h" />
<ClInclude Include="..\sakura_core\mem\CNativeT.h" />
Expand Down
3 changes: 3 additions & 0 deletions sakura/sakura.vcxproj.filters
Original file line number Diff line number Diff line change
Expand Up @@ -1088,6 +1088,9 @@
<ClInclude Include="..\sakura_core\recent\CRecentExcludeFolder.h">
<Filter>Cpp Source Files\recent</Filter>
</ClInclude>
<ClInclude Include="..\sakura_core\mem\CMemPool.h">
<Filter>Cpp Source Files\mem</Filter>
</ClInclude>
</ItemGroup>
<ItemGroup>
<None Include="..\sakura_core\Funccode_x.hsrc">
Expand Down
8 changes: 4 additions & 4 deletions sakura_core/doc/logic/CDocLineMgr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,15 +68,15 @@ CDocLineMgr::~CDocLineMgr()
//! pPosの直前に新しい行を挿入
CDocLine* CDocLineMgr::InsertNewLine(CDocLine* pPos)
{
CDocLine* pcDocLineNew = new CDocLine;
CDocLine* pcDocLineNew = m_memPool.Construct();
_InsertBeforePos(pcDocLineNew,pPos);
return pcDocLineNew;
}

//! 最下部に新しい行を挿入
CDocLine* CDocLineMgr::AddNewLine()
{
CDocLine* pcDocLineNew = new CDocLine;
CDocLine* pcDocLineNew = m_memPool.Construct();
_PushBottom(pcDocLineNew);
return pcDocLineNew;
}
Expand All @@ -87,7 +87,7 @@ void CDocLineMgr::DeleteAllLine()
CDocLine* pDocLine = m_pDocLineTop;
while( pDocLine ){
CDocLine* pDocLineNext = pDocLine->GetNextLine();
delete pDocLine;
m_memPool.Destruct(pDocLine);
pDocLine = pDocLineNext;
}
_Init();
Expand Down Expand Up @@ -118,7 +118,7 @@ void CDocLineMgr::DeleteLine( CDocLine* pcDocLineDel )
}

//データ削除
delete pcDocLineDel;
m_memPool.Destruct(pcDocLineDel);

//行数減算
m_nLines--;
Expand Down
4 changes: 3 additions & 1 deletion sakura_core/doc/logic/CDocLineMgr.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,9 @@
#include "basis/SakuraBasis.h"
#include "util/design_template.h"
#include "COpe.h"
#include "CDocLine.h"
#include "mem/CMemPool.h"

class CDocLine; // 2002/2/10 aroka
class CBregexp; // 2002/2/10 aroka

struct DocLineReplaceArg {
Expand Down Expand Up @@ -89,6 +90,7 @@ class CDocLineMgr{
CDocLine* m_pDocLineTop; //!< 最初の行
CDocLine* m_pDocLineBot; //!< 最後の行(※1行しかない場合はm_pDocLineTopと等しくなる)
CLogicInt m_nLines; //!< 全行数
CMemPool<CDocLine> m_memPool;

public:
//$$ kobake注: 以下、絶対に切り離したい(最低切り離せなくても、変数の意味をコメントで明確に記すべき)変数群
Expand Down
142 changes: 142 additions & 0 deletions sakura_core/mem/CMemPool.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
/*
Copyright (C) 2018-2019 Sakura Editor Organization

This software is provided 'as-is', without any express or implied
warranty. In no event will the authors be held liable for any damages
arising from the use of this software.

Permission is granted to anyone to use this software for any purpose,
including commercial applications, and to alter it and redistribute it
freely, subject to the following restrictions:

1. The origin of this software must not be misrepresented;
you must not claim that you wrote the original software.
If you use this software in a product, an acknowledgment
in the product documentation would be appreciated but is
not required.

2. Altered source versions must be plainly marked as such,
and must not be misrepresented as being the original software.

3. This notice may not be removed or altered from any source
distribution.
*/

#ifndef SAKURA_CMEMPOOL_H_
#define SAKURA_CMEMPOOL_H_

#include <memory>
#include "util/design_template.h"

// T : 要素型
// BlockSize : ブロックの大きさのバイト数
template <typename T, size_t BlockSize = 4096>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

このクラスに対して、単体テストを実装してほしいです。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

追加しました。クラスのI/Fを意図的にミニマムにしたので試験内容も単純なものになっています。

class CMemPool final
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

データ構造やロジックの説明をコメントに追加してほしいです。
内部仕様と、外部仕様も。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

コメントを記載しました。

{
public:
DISALLOW_COPY_AND_ASSIGN(CMemPool);

CMemPool()
{
// 始めのブロックをメモリ確保
AllocateBlock();
}

~CMemPool()
{
// メモリ確保した領域の連結リストを辿って全てのブロック分のメモリ解放
Block* curr = m_currentBlock;
while (curr) {
Block* next = curr->next;
operator delete(curr);
curr = next;
}
}

// 要素構築、引数は要素型のコンストラクタ引数
template <typename... Args>
T* Construct(Args&&... args)
{
T* p = Allocate();
new (p) T(std::forward<Args>(args)...);
return p;
}

// 要素破棄、引数は Construct が返したポインタ
void Destruct(T* p)
{
if (p) {
p->~T();
Free(p);
}
}

private:

union Node {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ここでなぜ union を使用しているのですか?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

下記の2つの理由が考えられます。

  • 領域を再利用する為に要素型と自己参照用のポインタを同じ領域に割り当てる
  • 共用体のサイズは各メンバを格納できるサイズになる事を利用

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

自己参照用のポインタと要素型を同じ領域に配置して問題ないのですか?
可能であれば、アスキーアート(あるいは SVG とか)でデータ構造の説明が欲しいです。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

134行目の
body = std::align(alignof(Node), sizeof(Node), body, space); からすると

こんな感じ?


     +----------------------------------+
     |                                  V
+-----------+---------+------------+-----------+---------+------------+
| ポインタ  |         |            | ポインタ  |         |            |
|   next    | padding | T のデータ |   next    | padding | T のデータ |
|           |         |            |           |         |            |
+-----------+---------+------------+-----------+---------+------------+
                      ^                 |                                 ^
                      |                 +---------------------------------+
              タイプ T のサイズ向けに align

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

自己参照用のポインタは要素が破棄されて未割当領域になった際に使われるので問題無い認識です。ブロック先頭部分のポインタはブロック間を接続するのに使われて、先頭部分に要素は記録されない為問題無いです。

ブロックを構造体型として定義した方がコードの見通しが良くなるかもしれないですね。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@m-tmatma さん

可能であれば、アスキーアート(あるいは SVG とか)でデータ構造の説明が欲しいです。

図をPlantUMLとかで用意する事は出来ますが、行動に移すのを躊躇する何かが有ります。

今回作成したメモリプールの実装は100行未満の低機能な単純なものなので、分かりやすいコードの書き方にする事に注力した方が良いかも知れません。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

別に図にしなくても構造を説明する文章でもいいですが
第三者が理解できる説明でもいいです。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

現状のコード読んで作りが理解できないですか?それならこのPR自体諦めますけど。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

コメントを書いてほしい理由は、初めて見た人が
パッと構造を理解できる助けになるようにして
ほしいからです

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

コメントは不要です。

~Node() {}
T element; // 要素型
Node* next; // 解放後の未割当領域の場合は次の未割当領域に繋がる
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

element が有効か、next が有効かはどんなロジックで
判断されますか?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • element として扱う場合(next としては扱わない)
    • CMemPool::Construct メソッドが呼び出されるとそこから CMemPool::Allocate プライベートメソッドを呼び出して割り当てる領域(Node)の要素(Element)のアドレスを返します。それ以降は破棄されるまではその領域は要素として扱われます。
  • next として扱う場合(element としては扱わない)
    • CMemPool::Destruct メソッドが呼び出されると要素のデストラクタ呼び出しを行って破棄し、Nodeを未割当の領域として扱うようになります。要素として扱わなくなった時点で領域 を自己参照型として扱えるようになります。

Copy link
Member

@m-tmatma m-tmatma Aug 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

観点が逆です。
element として扱う場合とnext として扱う場合の
場合分けをする判断基準が知りたいです。

心配してるのは、element のデータを next とみなしたり
その逆になるパターンがないか?です

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

もしそういうパターンに陥るケースがある事に気づいたりコードにロジック上の不備を見つけたのならそれを指摘して下さい。延々と体裁とかに関する要求をされて不毛な感じです。

メモリプールクラスを汎用的にして品質を高める事に注目したいわけではなくて、パフォーマンスを改善したかったんですが…。

sakura-editor/management-forum#8

を再度読んでください。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

コメントを書いてほしいというのは
別に体裁に関するコメントではありません。

@beru さんの考えているより、このクラスは
簡単ではないと思います。

最初にデータ構造をしめす説明をするなり
コメントなりがあるとレビューする側の時間を
節約できます。

PRを作成する人は、多くの時間を割いていますが、
同時に レビューする側も同様に時間をかけています。

レビューする側の時間を節約するという観点も
持って頂けると有難いです。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

コメントで説明をごてごてに書く事よりコードの記述に注力しないと結局出来上がるものに寄与しません。

データ構造の説明って最初からメモリプールって書いてるじゃないですか…。コード読んで分からなかったら頭の中でシミュレートしたり実際にビルドして動かしてロジックの内容をつかんでからレビューコメント書くべきですよ。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

コメントで説明をごてごてに書く事よりコードの記述に注力しないと結局出来上がるものに寄与しません。

まず認識していだだきたいのは、レビューに時間を割いているのは
PR に期待しているからです。

大して価値のない PR ならレビューに時間はかけないです。
価値のあるものだから、よりよくしようと意見を言うわけです。

コメントというのは単に手段であって目的ではないです。
コメントという形をとっていますが、どちらかというと
設計および仕様を残してほしいという意図です。

この PR がマージされたあと、コードを書いた人とは別の人が容易に
変更できることを目的にしてコメントを書いてほしいといっています。

データ構造の説明って最初からメモリプールって書いてるじゃないですか…。

メモリプールといってもどういう構造をとるかはいろいろあると思います。

コード読んで分からなかったら頭の中でシミュレートしたり実際にビルドして動かしてロジックの内容をつかんでからレビューコメント書くべきですよ。

コメントを書くのは嫌なのは想像しますが、
レビューアーがレビューしやすいような PR を作成するのは重要ですよ。
そうすることで早くマージされる可能性が上がるとおもいます。

メモリプールクラスを汎用的にして品質を高める事に注目したいわけではなくて、パフォーマンスを改善したかったんですが…。

パフォーマンスを改善するために、バグを生んでは元も子もないので、
基本的な部分で使われるクラスに関しては、バグのないようにするのか
基本かと思います。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

観点が逆です。
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 に入れるとかでしょうか?

さてやたらと回答が長文になってしまいましたが、文章力が貧弱なせいかもしれません。とほほー。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

まず認識していだだきたいのは、レビューに時間を割いているのは
PR に期待しているからです。

大して価値のない PR ならレビューに時間はかけないです。
価値のあるものだから、よりよくしようと意見を言うわけです。

ありがとうございます。

コメントというのは単に手段であって目的ではないです。
コメントという形をとっていますが、どちらかというと
設計および仕様を残してほしいという意図です。

ソースコードも設計および仕様の一形態ですが、C/C++は形式言語ではないですし静的解析も行いにくいのでそれだけでは限界があるかもしれないですね。

@m-tmatma さんの要求って、レビューアーが細部を理解出来る事は当然として、バックグラウンドが様々な第三者が見ても内容がすっきりと分かるくらいの説明を残してほしいという事ですよね?それは理想的かもしれないですけど、そこまで本当に必要なんでしょうか?

分かる人には分かる、的なコードばっかりだとメンテが確かにきつくなるとは思いますけど、ドキュメント文化におんぶにだっこしたがりマンばっかりだとそれもなぁ…。

この PR がマージされたあと、コードを書いた人とは別の人が容易に
変更できることを目的にしてコメントを書いてほしいといっています。

なんだかその調子で行くと、拡張性が足りない、汎用性が無い、とか色々な要求も上乗せされそうですね…。

メモリプールといってもどういう構造をとるかはいろいろあると思います。

そりゃそうでしょうけど、教材を作っているんじゃないんだし懇切丁寧なコメントとテストとドキュメントを揃えないと取り込めない、ってなるとキッツいです。

コメントを書くのは嫌なのは想像しますが、
レビューアーがレビューしやすいような PR を作成するのは重要ですよ。
そうすることで早くマージされる可能性が上がるとおもいます。

そうですねぇ。レビューアーは審査官みたいなものですね。

パフォーマンスを改善するために、バグを生んでは元も子もないので、
基本的な部分で使われるクラスに関しては、バグのないようにするのか
基本かと思います。

私も他力本願ですが、レビューアーがコメントが無いコードを読解してバグを指摘してくれたら楽だなぁ、と思います。

};

union Block {
~Block() {}
struct {
Block* next;
Node nodes[1];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

もともとのコードではここに padding 入れてませんでしたか?
コンパイラにお任せですか?

Copy link
Contributor Author

@beru beru Aug 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

はい。もともとのコードでは、CMemPool::AllocateBlock メソッドで std::align 関数を使ってアライメントを取っていましたが、構造体を使う事でコンパイラに任せるようにしました。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

static_assert 等などで、それを保証した方がいいと思います

Copy link
Contributor Author

@beru beru Aug 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

変数のアドレスは動作時に決まるものなので、静的には保証出来ないのではないかと思い当たりました。

なので行うべきは CMemPool::AllocateBlock プライベートメソッドにおいて operator new で確保した領域を Block* にキャストして m_currentBlock メンバーに割り当てる際に、std::align を使って確実にアライメントが取られるようにする事、な気がします。

単体テストでももっと大きいアライメントを要求する型を用意して確認する事にします。

};
uint8_t padding[BlockSize];
};

// 要素のメモリ確保処理、要素の領域のポインタを返す
T* Allocate()
{
// メモリ確保時には未割当領域から使用していく
if (m_unassignedNode) {
T* ret = &m_unassignedNode->element;
m_unassignedNode = m_unassignedNode->next;
return ret;
}
else {
// 未割当領域が無い場合は、ブロックの中のNode領域を使用する
if (reinterpret_cast<void*>(m_currentNode + 1) >= reinterpret_cast<void*>(m_currentBlock + 1)) {
// 現在のブロックに新規に割り当てるNode分の領域が残っていない場合は新規にブロックを確保
AllocateBlock();
}
T* ret = &m_currentNode->element;
++m_currentNode;
return ret;
}
}

// メモリ解放処理、要素の領域のポインタを受け取る
// Allocate メソッドで返したポインタを渡す事
void Free(T* p)
{
if (p) {
// メモリ解放した領域を未割当領域として自己参照共用体の片方向連結リストで繋げる
// 次回のメモリ確保時にその領域を再利用する
Node* next = m_unassignedNode;
m_unassignedNode = reinterpret_cast<Node*>(p);
m_unassignedNode->next = next;
}
}

// 呼び出しの度にメモリの動的確保を細かく行う事を避ける為に、一括でブロック領域を確保
void AllocateBlock()
{
Block* next = m_currentBlock;
// 以前に作成したブロックに連結する
m_currentBlock = reinterpret_cast<Block*>(operator new(sizeof(Block)));
m_currentBlock->next = next;

// 新規に作成したブロックの先頭のNodeから使用する
m_currentNode = &m_currentBlock->nodes[0];
}

Block* m_currentBlock = nullptr; // 現在のブロック
Node* m_unassignedNode = nullptr; // 未割当領域の先頭
Node* m_currentNode = nullptr; // 現在のブロックの中のNodeを指すポインタ、メモリ確保時に未割当領域が無い場合はここを使う
};

#endif /* SAKURA_CMEMPOOL_H_ */
171 changes: 171 additions & 0 deletions tests/unittests/test-cmempool.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
/*! @file */
/*
Copyright (C) 2018-2019 Sakura Editor Organization

This software is provided 'as-is', without any express or implied
warranty. In no event will the authors be held liable for any damages
arising from the use of this software.

Permission is granted to anyone to use this software for any purpose,
including commercial applications, and to alter it and redistribute it
freely, subject to the following restrictions:

1. The origin of this software must not be misrepresented;
you must not claim that you wrote the original software.
If you use this software in a product, an acknowledgment
in the product documentation would be appreciated but is
not required.

2. Altered source versions must be plainly marked as such,
and must not be misrepresented as being the original software.

3. This notice may not be removed or altered from any source
distribution.
*/
#include <gtest/gtest.h>
#include "mem/CMemPool.h"
#include <vector>
#include <array>
#include <string>
#include <type_traits>
#include <stdint.h>
#include <Windows.h>
m-tmatma marked this conversation as resolved.
Show resolved Hide resolved

template <typename T>
class CMemPoolTest : public ::testing::Test{};

// BlockSize テンプレート引数のバリエーション用意
using test_types = ::testing::Types<
std::integral_constant<std::size_t, 1>,
std::integral_constant<std::size_t, 2>,
std::integral_constant<std::size_t, 3>,
std::integral_constant<std::size_t, 4>,
std::integral_constant<std::size_t, 8>,
std::integral_constant<std::size_t, 16>,
std::integral_constant<std::size_t, 32>,
std::integral_constant<std::size_t, 64>,
std::integral_constant<std::size_t, 128>,
std::integral_constant<std::size_t, 256>,
std::integral_constant<std::size_t, 512>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BlockSize は 2の累乗である必要はありますか?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2の累乗値である必要はありません。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

だとしたら、そのテストもあったほうがいいと思います。

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2の累乗値ではない値も追加しました。

std::integral_constant<std::size_t, 1024>,
std::integral_constant<std::size_t, 2048>,
std::integral_constant<std::size_t, 4096>,
std::integral_constant<std::size_t, 8192>,
std::integral_constant<std::size_t, 16384>,
std::integral_constant<std::size_t, 32768>,
std::integral_constant<std::size_t, 65536>
>;

TYPED_TEST_CASE(CMemPoolTest, test_types);

// ポインタのアライメントが取れているか確認
#define is_aligned(POINTER, BYTE_COUNT) (((uintptr_t)(const void *)(POINTER)) % (BYTE_COUNT) == 0)

// デフォルトコンストラクタを指定回数呼び出し
template <typename T, size_t BlockSize>
void testPool(size_t sz)
{
CMemPool<T, BlockSize> pool;
std::vector<T*> ptrs(sz);
// 要素構築
for (size_t i=0; i<sz; ++i) {
T* p = pool.Construct();
ptrs[i] = p;
ASSERT_TRUE(p != nullptr); // 領域を確保できたか確認
ASSERT_TRUE(is_aligned(p, alignof(T))); // アライメントが取れているか確認
}
// 要素破棄
for (size_t i=0; i<sz; ++i) {
pool.Destruct(ptrs[i]);
}
// 要素破棄後に要素を再度構築
for (size_t i=0; i<sz; ++i) {
T* p = pool.Construct();
ptrs[i] = p;
ASSERT_TRUE(p != nullptr); // 領域を確保できたか確認
ASSERT_TRUE(is_aligned(p, alignof(T))); // アライメントが取れているか確認
}
// 再度要素を破棄
for (size_t i=0; i<sz; ++i) {
pool.Destruct(ptrs[i]);
}
}

// 色々な型やBlockSizeでデフォルトコンストラクタをたくさん呼び出す
TYPED_TEST(CMemPoolTest, default_constructor)
{
// BlockSize を取得
constexpr size_t BlockSize = TypeParam::value;

// ユーザー定義型の確認用
struct TestStruct
{
int* ptr;
char buff[32];
long long ll;
double f64;
};

/*!
色々なケースの確認
*/
size_t sz = 1;
for (size_t i=0; i<10; ++i, sz*=2) {
testPool<int8_t, BlockSize>(sz);
testPool<int16_t, BlockSize>(sz);
testPool<int32_t, BlockSize>(sz);
testPool<int64_t, BlockSize>(sz);
testPool<float, BlockSize>(sz);
testPool<double, BlockSize>(sz);
testPool<FILETIME, BlockSize>(sz);
testPool<TestStruct, BlockSize>(sz);
testPool<std::wstring, BlockSize>(sz);
}
}

// 引数付きコンストラクタ
TYPED_TEST(CMemPoolTest, parameterized_constructor)
{
constexpr size_t BlockSize = TypeParam::value;
CMemPool<std::wstring, BlockSize> pool;

std::wstring* p0 = nullptr;
std::wstring* p1 = nullptr;
std::wstring* p2 = nullptr;
std::wstring* p3 = nullptr;

std::wstring s0{L"あいうえお"};
std::wstring s1{L"nullptr"};
wchar_t c2{'㌍'};
size_t len2 = 12345;
std::wstring s2(len2, c2);
std::wstring s3{L"令和"};

// 構築と破棄を繰り返して動作するか確認
for (size_t i=0; i<8; ++i) {
// 要素構築
p0 = pool.Construct(s0);
p1 = pool.Construct(s1);
p2 = pool.Construct(len2, c2);
p3 = pool.Construct(s3);

// 領域を構築できたか確認
ASSERT_TRUE(p0 != nullptr);
ASSERT_TRUE(p1 != nullptr);
ASSERT_TRUE(p2 != nullptr);
ASSERT_TRUE(p3 != nullptr);

// コンストラクタに指定した値と同一か確認
ASSERT_TRUE(*p0 == s0);
ASSERT_TRUE(*p1 == s1);
ASSERT_TRUE(*p2 == s2);
ASSERT_TRUE(*p3 == s3);

// 要素破棄
pool.Destruct(p0);
pool.Destruct(p1);
pool.Destruct(p2);
pool.Destruct(p3);
}
}