Bug（バグ） #3445
完了
Task（タスク） #3403: OpenPNE の既存のテストコードが正常に動作するように修正を行う
#927 の修正によって無関係なCommunityMemberインスタンスが解放される問題の回避
0%
説明
Overview (現象)¶
Doctrine_Record::free() メソッドには https://gist.github.com/upsilon/6896385 のような Doctrine のバグと思わしき現象が存在している。
#927 でのメモリリーク改善のために行った d6956cde880be65aefc802e01b070bb9d054145f の修正は CommunityTableTest.php などのテストコードでこの現象を引き起こしており、例えば下記のようなコードを実行するとエラーが発生する。
$communityMember1 = Doctrine_Core::getTable('CommunityMember')
->findOneByCommunityIdAndMemberId(1, 1);
Doctrine_Core::getTable('CommunityMember')->isMember(1, 1);
print $communityMember1->community_id; // エラー
Way to fix (修正内容)¶
この問題を根本的に解決するためには Doctrine 自体に手を加える必要があるため、ここでは不具合の回避のための修正を行う。具体的には #927 で行ったメモリリーク改善の効果を落とさないように CommunityMember オブジェクトに対する free() メソッドの呼び出しを最小限に抑えるための修正を施す。
Youichi Kimura さんが約11年前に更新
- ステータス を Accepted（着手） から Pending Review（レビュー待ち） に変更
- 進捗率 を 0 から 50 に変更
更新履歴 aebd70f001af18f67cbcbcf9dbdda0cbac853e1c で適用されました。
Shinichi Urabe さんが約11年前に更新
- ステータス を Pending Review（レビュー待ち） から Rejected（差し戻し） に変更
修正を確認しました。修正自体は問題ないと思います。
管理画面からの特定コミュニティへの全員参加ボタンの動作の処理速度を念のために確認しました。
- 条件
- 1040名のユーザを特定のコミュニティに一括参加
- xhprof 0.9.3 で計測
- 環境 php 5.3.27
- 修正前
| Total Incl. Wall Time (microsec) | 12,627,795 microsecs |
|---|---|
| Total Incl. PeakMemUse (bytes) | 8,040,072 bytes |
| Number of Function Calls | 1,936,440 |
- 修正後
| Total Incl. Wall Time (microsec) | 10,833,694 microsecs |
|---|---|
| Total Incl. PeakMemUse (bytes) | 9,361,328 bytes |
| Number of Function Calls | 1,773,829 |
差し戻し点¶
CommunityMemberPositionTable::getPositionsByMemberIdAndCommunityId() 内の処理で Doctrine_Query::fetchArray() を使うように変更されましたが、hydration mode を Doctrine_Core::HYDRATE_NONE にした方が効果的ですので、そちらを使うようにしてください。
Youichi Kimura さんが約11年前に更新
- ステータス を Rejected（差し戻し） から Pending Review（レビュー待ち） に変更
更新履歴 da55e3f947c741751a9f30de51710b67b9ca70ed で適用されました。
Youichi Kimura さんが約11年前に更新
da55e3f9 について:
Doctrine_Core::HYDRATE_NONE を使用した場合に、下記のようにメソッドが実現する処理に対して記述が冗長になることや Doctrine_Query を使用するメリットが見出だせなかったため、Doctrine_Connection で直接 SQL クエリを実行する修正にしました。
public function getPositionsByMemberIdAndCommunityId($memberId, $communityId)
{
$query = $this->createQuery()
->select('name')
->where('member_id = ?', $memberId)
->andWhere('community_id = ?', $communityId)
->setHydrationMode(Doctrine_Core::HYDRATE_NONE);
$results = array();
foreach ($query->execute() as $row)
{
$results[] = $row[0];
}
$query->free();
return $results;
}
Shinichi Urabe さんが約11年前に更新
- ステータス を Pending Review（レビュー待ち） から Rejected（差し戻し） に変更
Youichi Kimura は書きました:
da55e3f9 について:
Doctrine_Core::HYDRATE_NONE を使用した場合に、下記のようにメソッドが実現する処理に対して記述が冗長になることや Doctrine_Query を使用するメリットが見出だせなかったため、Doctrine_Connection で直接 SQL クエリを実行する修正にしました。
[...]
コメントありがとうございます。
こちらですが、記述は確かに冗長になりますが、OpenPNE のソースで Doctrine::HYDRATE_NONE という文字で検索してもらえれば、チューニングの対応として使っていることが確認できるかと思います。
また、一部あるようですが、OpenPNE のソース上で SQL を直書きしている箇所は少ないのと、SQLで書くと event listener が使えなくなりますので、特別な理由がなければ、可能な限り Doctrine_Query を使った方がよいと思います。
Youichi Kimura さんが約11年前に更新
- ステータス を Rejected（差し戻し） から Pending Review（レビュー待ち） に変更
更新履歴 27853bc54918d78ae098fef478e9e971448f04b1 で適用されました。
Shinichi Urabe さんが約11年前に更新
- ステータス を Pending Review（レビュー待ち） から Pending Testing（テスト待ち） に変更
- 進捗率 を 50 から 70 に変更
レビューOKです
