Hi,

Found out during profiling of a site that uses global_filter that global_filter_get_field_labels() was taking up a large proportion of cpu time on page load. This function seems to take a list of *all* field instances on the site and load each one in turn and not just the instances that relate to the field being used by the filter.

One easy win I found was to only load the field instance when a $field_type is passed in - as it only needs to load the field in order to do a check against $field_type. As the function isn't being called with a $field_type this hugely improves performance. I will attach a patch.

An additional improvement would be that the field_info_instances() only returns instances for the field being used by the filter. Unfortunately I didn't have time to dig in and work that out.

Thanks for a great module.

Cheers,
Matt

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Matwilk created an issue. See original summary.

Matwilk’s picture

Hi,

Here's the patch I made.

Cheers,
Matt

joelpittet’s picture

Version: 7.x-1.13 » 7.x-1.x-dev
Status: Active » Needs review

Setting the branch and status for people review the patch and apply to the latest version.

joelpittet’s picture

Priority: Normal » Major
Issue tags: +Performance
FileSize
2.41 KB

@Matwilk, that is a great patch. Maybe we can take it a bit further. If a site is not even using fields and is only using a view, this gets called anyways! And not only that it gets called for every block on your page!

This patch keeps the existing functionality, but most of the functionality it provides is really for the block title/subject. That's a lot of work(loading all fields and views) for little gain.

  1. First thing, if it's not a 'global_filter_' $delta in the block_view, it stops trying to do stuff. That makes the hook_block_view() better for all blocks.
  2. Next it sets some statics up for the usage variables to prevent calling them more than once if they are needed at all.
  3. Then it checks if the filter even needs views before filling that empty static or fields.

So if you are using views, it won't even do the field info lookup, and visaversa. AND if it does at all, it will only do it once.

Yuri’s picture

Has this been committed yet? Rather important that this gets resolved.

joelpittet’s picture

Nope, the patch needs to be reviewed and tested (you can do this). And change the status of the issue to that.