Hi

ctools_entity_field_content_type_content_types() calls in our case 474 times to t() and that hurts. Yes we have loads of fields and entities and we can't cut them away.

The real problem is that the t() strings are gathered even if we just render a pane without needing those strings.

Maybe a solution could be to split the functionality? I see in the stack that in my case we come from ctools_content_render() so there might be an indicator when to create the lables and when not?

Comments

maximpodorov’s picture

Status: Active » Needs review
StatusFileSize
new888 bytes

This patch add caching for the information calculated in ctools_entity_field_content_type_content_types(). It's used in my project and reduces amount of t() calls noticeably.

japerry’s picture

Status: Needs review » Reviewed & tested by the community

Ooh! I like the cache set on here, works good marking RTBC. See if anyone has objections, if not I'll commit it.

maximpodorov’s picture

I have several more caching patches for ctools. :)

kars-t’s picture

Hi

thanks for the patch and code wise it looks fine. But aren't we missing some cache clear on change of the content types?

maximpodorov’s picture

Maybe. :)

mrjmd’s picture

japerry’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +SprintWeekend2015

Kars-T is correct, we need to add the ability to clear the cache for it before I commit.

maximpodorov’s picture

Status: Needs work » Needs review
StatusFileSize
new2.16 KB

I updated the patch to clear cache upon field crud operations.
The cache is also cleared when drupal_flush_all_caches() function is called (e.g. when the 'Clear all caches' button on the 'Performance' page is pressed).

BTW, the same caching can be used for entity_field_extra plugin.

damienmckenna’s picture

This didn't end up being included in 1.6.

maximpodorov’s picture

Unfortunately.

joelpittet’s picture

Status: Needs review » Reviewed & tested by the community
StatusFileSize
new145.47 KB

Saves ~50 ms off my homepage load. And remove > 480 calls to the t().

dillix’s picture

Why this excluded from 1.6?

joelpittet’s picture

@dillix because nobody RTBC'd it before the release was made.

dillix’s picture

Version: 7.x-1.5 » 7.x-1.6
joelpittet’s picture

Version: 7.x-1.6 » 7.x-1.x-dev

thanks, yeah this should be against dev.

joseph.olstad’s picture

++1 for commit this and tag a 1.7 release.

Well done maximpodorov

Performance improvements like this one are worth a minor version . CPU cycles and IO saved from this patch allow people to get work done faster!

I recently spent a thousand dollars on hardware upgrades because I didn't have this patch and a few others that I recently discovered. In the past 2 months I've been made aware of about 5 patches to various contrib modules and drupal core that made a far bigger performance improvement than the thousand dollars worth of hardware upgrades.

This patch saves CPU cycles which saves electricity , which saves the environment, saves peoples time and saves money. It's well worth a minor version.

maximpodorov’s picture

@joseph.olstad, what are these patches? I'd like to improve my projects also. :)

joelpittet’s picture

If you haven't seen this wiki it may be fun.

https://groups.drupal.org/node/210683

maximpodorov’s picture

Thank you.

dillix’s picture

I'm also waiting for this being committed. My projects got performance and works well with this patch about a week.

joseph.olstad’s picture

Issue tags: +Performance
japerry’s picture

Status: Reviewed & tested by the community » Fixed

Thanks everyone on this! Fixed.

Status: Fixed » Closed (fixed)

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