In a site with 43,000 users where each user is a group and can reference other groups (the user's trusted contacts), we experienced page generation times of ~15-20 seconds, apparently resulting from large numbers of entity loads. It appears that OG's OgBehaviorHandler->load() implementation may cause Entityreference to perform unnecessary entity loads on all of the referenced entities, when I believe all we need are the entity IDs.
The attached patch changes this behavior and brought page generation times for the same pages down to 1-2 seconds.
Marking "needs review" even though this patch fails some OG tests. At first glance, looks like these tests *may* be failing because the test are written assuming a select widget rather than an autocomplete one, though I believe this shouldn't be relevant to the load() function.
It would be great to get feedback on how we can improve the patch to be contrib-friendly.
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | 2169099-og-prevent-load-8.patch | 6.72 KB | ezra-g |
| #7 | 2169099-og-prevent-load-7.diff | 7.77 KB | amitaibu |
| #4 | 2169099-og-prevent-load-4.diff | 6.1 KB | amitaibu |
| og-ER-behavior-identifier-0.patch | 847 bytes | ezra-g |
Comments
Comment #2
amitaibuThe approach seems right, I believe it's now figuring out the reasons of the fails in the tests
Comment #3
ezra-g commentedSome information that may be related to this issue, we've also observed that when doing a user_load on a user, we see the og_get_group_members_properties() callback running and generating an EFQ that includes all the results of a user's trusted contacts. This makes sense, but it's not entirely clear what factors control when OG loads those referenced users versus when it just loads the OG memberships. I suspect it loads the memberships and then, without the patch in this issue,the user entities.
Comment #4
amitaibuLets see if the testbot likes it
Comment #6
amitaibuWe are down to a single failing test, so that's a good sign :)
Comment #7
amitaibuComment #8
ezra-g commentedRe-rolled to remove extraneous og.install changes.
Comment #9
amitaibuCommitted, thanks.
Comment #11
tame4tex commentedI believe that the changes to OgBehaviorHandler.class.php made in this commit may be causing an issue, specifically the following line in public function load():
$gids = og_get_entity_groups($entity_type, $entity, array(OG_STATE_ACTIVE), $field_name);I have a node content type "Client" which is a Group. Users can join the "Client" groups.
When a user's group membership is in the "blocked" or "pending" state, when editing a user account the field og_user_node is empty. This is because the OgBehaviorHandler class Load function only returns Active memberships. If a change is made to the user account and the user is saved, because og_user_node is empty the user is are completely removed from the Group and no longer visible on group/node/###/admin/people.
Is there a reason we have to specify OG_STATE_ACTIVE? Can it be changed to:
$gids = og_get_entity_groups($entity_type, $entity, array(), $field_name);Comment #12
amitaibuUpdate: wrong comment, deleted
Comment #13
amitaibu@tame4tex,
Care to roll a patch. Extra points for a test to prevent regression.
Comment #14
amitaibuI've opened #2224509: Fix loading only active memberships, so closing this issue.
Comment #15
tame4tex commentedThanks for the quick response Amitaibu! I have been offline for a couple of days and see Ezra has beaten me to the punch :-)