og_get_all_group_bundle and og_get_all_group_bundle can be very slow if the need to search through all bundles on all entity_types.

Most times when we call one of these two API functions we are only concerned with the group / group_content bundles that are part of the node entity type. The forthcoming patch will add an optional $entity_type parameter to these functions to prevent cycling over all system bundles.

It just so happens that we have 1k vocabularies that are all iterated over by this function, this changes page load times to 7 seconds.

CommentFileSizeAuthor
#8 interdiff-2308279-3-8.txt2.04 KBjacob.embree
#8 og-entity_type-parameter-2308279-8.patch3.35 KBjacob.embree
#3 add_entity_type_parameter-2308279-3.patch2.26 KBrbrandon
#1 add_entity_type_parameter-2308279-1.patch2.25 KBrbrandon
Screenshot 2014-07-23 09.27.43.png18.53 KBrbrandon
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rbrandon’s picture

rbrandon CreditAttribution: rbrandon commented
FileSize
add_entity_type_parameter-2308279-1.patch2.25 KB

Adds optional $entity_type parameter

rbrandon’s picture

rbrandon CreditAttribution: rbrandon commented
Status: Active » Needs review
rbrandon’s picture

rbrandon CreditAttribution: rbrandon commented
FileSize
add_entity_type_parameter-2308279-3.patch2.26 KB

Forgot default parameter for group_content_bundles. Corrected patch.

The last submitted patch, 1: add_entity_type_parameter-2308279-1.patch, failed testing.

rbrandon’s picture

rbrandon CreditAttribution: rbrandon commented

This was a big help for us, given the number of bundles we have. https://github.com/openscholar/openscholar/issues/6429#issuecomment-4992...

-Richard

shushu’s picture

shushu CreditAttribution: shushu commented
Status: Needs review » Reviewed & tested by the community
amitaibu’s picture

amitaibu
Primary language Hebrew
Location Israel
CreditAttribution: amitaibu commented
Status: Reviewed & tested by the community » Needs work

Hi @rbrandon :)

  1. +++ b/og.module
@@ -2475,18 +2475,29 @@ function og_get_all_group_entity() {
+function og_get_all_group_bundle($entity_type = NULL) {

    PHPdocs. 

  2. +++ b/og.module
@@ -2475,18 +2475,29 @@ function og_get_all_group_entity() {
+  } else {

    }
else {

  3. +++ b/og.module
@@ -2475,18 +2475,29 @@ function og_get_all_group_entity() {
+  if (empty($entity_type)) {

    one liner.

jacob.embree’s picture

jacob.embree CreditAttribution: jacob.embree at St. Louis Integration commented
Title: Allow og_get_all_group_content_bundle and og_get_all_group_bundle to search a single entity_type » Allow og_get_all_group_content_bundle() and og_get_all_group_bundle to search a single entity_type()
Assigned: rbrandon » Unassigned
Status: Needs work » Needs review
FileSize
og-entity_type-parameter-2308279-8.patch3.35 KB
interdiff-2308279-3-8.txt2.04 KB

This patch fixes amitaibu's #2. Not sure what #1 and #3 are. Also, documentation improvements are made to the two affected functions.

jacob.embree’s picture

jacob.embree CreditAttribution: jacob.embree at St. Louis Integration commented
Title: Allow og_get_all_group_content_bundle() and og_get_all_group_bundle to search a single entity_type() » Allow og_get_all_group_content_bundle() and og_get_all_group_bundle() to search a single entity_type
torgosPizza’s picture

torgosPizza
he/him
Primary language English
Location Portland, OR
CreditAttribution: torgosPizza commented

Thanks for this patch! I might suggest a small addition, which is the ability for us to configure a default entity type for when the function og_entity_property_info() is called, for example:

+  $default_og_content_bundle = variable_get('og_default_content_type', NULL);
+
   // Add OG membership metadata for every bundle that is a group content.
-  foreach (og_get_all_group_content_bundle() as $entity_type => $bundles) {
+  foreach (og_get_all_group_content_bundle($default_og_content_bundle) as $entity_type => $bundles) {

My reasoning is that, in New Relic, I'm seeing og_entity_property_info() get called during cache-rebuild, which occasionally coincides with a customer visiting a page on the site. The resulting cache-setting process results in longer page load times due to the fact that we're currently scanning all entity types. At least this way we can set a default entity and content entity type (in our case, both are just `node`) to prevent extra processing time.