Bug（バグ） #3445
Task（タスク） #3403: OpenPNE の既存のテストコードが正常に動作するように修正を行う
#927 の修正によって無関係なCommunityMemberインスタンスが解放される問題の回避
0%
Description
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() メソッドの呼び出しを最小限に抑えるための修正を施す。
Related issues
Associated revisions
add the $free option for Community::joinAllMembers() (fixes #3445)
more improvements for getPositionsByMemberIdAndCommunityId() method (fixes #3445)
use HYDRATE_NONE mode for getPositionsByMemberIdAndCommunityId query (fixes #3445)
History
#1
Updated by Youichi Kimura about 11 years ago
Updated by - Description updated (diff)
- Status changed from New（新規） to Accepted（着手）
#2
Updated by Youichi Kimura about 11 years ago
- Status changed from Accepted（着手） to Pending Review（レビュー待ち）
- % Done changed from 0 to 50
更新履歴 aebd70f001af18f67cbcbcf9dbdda0cbac853e1c で適用されました。
#3
Updated by Youichi Kimura about 11 years ago
更新履歴 b5440f002e0925dbb0a10f659db1e8a85eea5d27 で適用されました。
#4
Updated by Shinichi Urabe about 11 years ago
Updated by - Status changed from Pending Review（レビュー待ち） to 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 にした方が効果的ですので、そちらを使うようにしてください。
#5
Updated by Youichi Kimura about 11 years ago
- Status changed from Rejected（差し戻し） to Pending Review（レビュー待ち）
更新履歴 da55e3f947c741751a9f30de51710b67b9ca70ed で適用されました。
#6
Updated by Youichi Kimura about 11 years ago
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;
}
#7
Updated by Shinichi Urabe about 11 years ago
- Status changed from Pending Review（レビュー待ち） to Rejected（差し戻し）
Youichi Kimura は書きました:
da55e3f9 について:
Doctrine_Core::HYDRATE_NONE を使用した場合に、下記のようにメソッドが実現する処理に対して記述が冗長になることや Doctrine_Query を使用するメリットが見出だせなかったため、Doctrine_Connection で直接 SQL クエリを実行する修正にしました。
[...]
コメントありがとうございます。
こちらですが、記述は確かに冗長になりますが、OpenPNE のソースで Doctrine::HYDRATE_NONE という文字で検索してもらえれば、チューニングの対応として使っていることが確認できるかと思います。
また、一部あるようですが、OpenPNE のソース上で SQL を直書きしている箇所は少ないのと、SQLで書くと event listener が使えなくなりますので、特別な理由がなければ、可能な限り Doctrine_Query を使った方がよいと思います。
#8
Updated by Youichi Kimura about 11 years ago
- Status changed from Rejected（差し戻し） to Pending Review（レビュー待ち）
更新履歴 27853bc54918d78ae098fef478e9e971448f04b1 で適用されました。
#9
Updated by Shinichi Urabe about 11 years ago
- Status changed from Pending Review（レビュー待ち） to Pending Testing（テスト待ち）
- % Done changed from 50 to 70
レビューOKです
#11
Updated by kaoru n over 7 years ago
Updated by - Status changed from Pending Testing（テスト待ち） to Won't fix（対応せず）
- Target version changed from OpenPNE 3.9.0-old to OpenPNE 3.9.0
- % Done changed from 70 to 0
OpenPNE 3.8.9 にて対応済みであったため、対応せずとします。