metatag_entity_load() uses metatag_entity_supports_metatags() for querying entities metatags instances, but it could reduce the $revision_ids by first checking if the bundles are enabled, thus potentially reducing the number of SQL queries.

Comments

pounard’s picture

Status:Active» Needs review
Issue tags:+Performance
StatusFileSize
new2.77 KB
PASSED: [[SimpleTest]]: [MySQL] 73 pass(es).
[ View ]

For example, the given patch applied on a module where lives something like 50 node types but only 4 of them are enabled to use metatags (using hook_entity_info_alter()) saves us an average of 30 SQL queries per page, which is significant.

pounard’s picture

Status:Needs review» Reviewed & tested by the community
StatusFileSize
new2.53 KB

And here is a patch for the current version.
Patch has been done with a manual diff, so you need to:

patch -p0 < PATCHFILE

manually to apply. Drush make will apply cleanly.

DamienMcKenna’s picture

Status:Reviewed & tested by the community» Needs review
DamienMcKenna’s picture

StatusFileSize
new3.07 KB
PASSED: [[SimpleTest]]: [MySQL] 73 pass(es).
[ View ]

@pounard: Thanks for the patch. I've made a few minor tweaks.

pounard’s picture

Looks good to me, glad I could help.

DamienMcKenna’s picture

Issue summary:View changes
StatusFileSize
new3.58 KB
PASSED: [[SimpleTest]]: [MySQL] 73 pass(es).
[ View ]

Rerolled.

DamienMcKenna’s picture

StatusFileSize
new3.62 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch metatag-n2070821-7.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Rerolled.

DamienMcKenna’s picture

Status:Needs review» Needs work

This needs work.

DamienMcKenna’s picture

Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, 7: metatag-n2070821-7.patch, failed testing.

DamienMcKenna’s picture

Status:Needs work» Needs review
Issue tags:-Performance
StatusFileSize
new23.04 KB
FAILED: [[SimpleTest]]: [MySQL] 98 pass(es), 3 fail(s), and 1 exception(s).
[ View ]

While I was rerolling this patch I realized something: A lot of work was being done to deal with the fact that you have to change the API of an entity to indicate it's supported by Metatag, and then there's a settings page which lets you control which one of those entity types is actually enabled. And then there was a whole extra function / internal API for deciding whether entity types should be disabled anyway. And if there wasn't an enabled global configuration for a specific entity there was no way of supporting that entity. It was all a little confusing.

So I've simplified this whole system:

  • The API to control whether entities are supported has been removed.
  • From now on, any entity that has view modes and is not a configuration entity may be enabled or disabled through the Advanced Settings page, with a default of "enabled" (files, comments and fieldable_panels_panes are disabled by default).
  • An update script is provided to move existing sites to this configuration.
  • An install hook is added to define some initial values.
  • The uninstall hook was updated accordingly.
  • There's now a function for enabling support for a specific entity type or bundle: metatag_entity_type_enable()
  • There's now a corresponding function for disabling support for a specific entity type or bundle: metatag_entity_type_disable()
  • All existing tests pass correctly! Woo!!
  • In my own personal testing I was able create meta tags for a node, a term and a user, so the basic functionality appears to still work correctly.
  • This will require a change notice.
  • Life will be simpler going forwards!
  • I don't think if this is really a performance improvement anymore ;-)

Please let me know what you think!

DamienMcKenna’s picture

StatusFileSize
new23.04 KB
FAILED: [[SimpleTest]]: [MySQL] 98 pass(es), 3 fail(s), and 1 exception(s).
[ View ]

Some minor adjustments - I accidentally renamed the "Advanced settings" fieldset on the (newly renamed) settings page, and improved the entity type settings' description a little on that page.

The last submitted patch, 12: metatag-n2070821-12.patch, failed testing.

Status:Needs review» Needs work

The last submitted patch, 13: metatag-n2070821-12.patch, failed testing.

DamienMcKenna’s picture

The tests pass locally but fail on testbot :(

DamienMcKenna’s picture

Status:Needs work» Needs review
StatusFileSize
new39.71 KB
PASSED: [[SimpleTest]]: [MySQL] 101 pass(es).
[ View ]

Well, if I upload the same patch twice then no wonder it fails ;-) Also, I'd accidentally created the patches in the wrong location, so the patch in #12 wasn't the correct one to start with.

*This* is the correct patch file.

DamienMcKenna’s picture

StatusFileSize
new40.97 KB
PASSED: [[SimpleTest]]: [MySQL] 101 pass(es).
[ View ]

This removes hook_entity_info_alter() entirely, clears the caches at the end of the update script and adds another update script to clear the menu cache because the 'advanced settings' page was renamed to just 'settings'.

  • DamienMcKenna committed b11b55d on 7.x-1.x
    Issue #2070821 by DamienMcKenna, pounard: Major re-architecture to how...
DamienMcKenna’s picture

Status:Needs review» Fixed

Committed. I'll need to do a change notice for this.

DamienMcKenna’s picture

I've written up a change notice: https://www.drupal.org/node/2495819

pounard’s picture

It took quite some time, but thank you very much for commiting this.

Status:Fixed» Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.