Problem/Motivation

Was doing some performance testing and noticing a high number of t() calls from views_fetch_fields(). They are all the same 5 strings being translated so we can build them 5 times and avoid the call 1000s of times.

Proposed resolution

Make the calls early and outside the nested loop.

CommentFileSizeAuthor
#12 reduce_the_calls_to_the-2760405-12.patch2.52 KBjoseph.olstad
#2 reduce_the_calls_to_the-2760405-2.patch2.51 KBjoelpittet
XHProf__views_fetch_fields.png245.41 KBjoelpittet

Comments

joelpittet created an issue. See original summary.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Issue summary: View changes
StatusFileSize
new reduce_the_calls_to_the-2760405-2.patch2.51 KB
joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Active » Needs review
joseph.olstad’s picture

joseph.olstad
Primary language French
commented
Status: Needs review » Reviewed & tested by the community

Works as advertised.

joseph.olstad’s picture

joseph.olstad
Primary language French
commented
Issue tags: +minor version target

joelpittet has done some great work here. Adding the 'minor version target' tag.
Please include this for 7.x-3.17

joseph.olstad’s picture

joseph.olstad
Primary language French
commented
Related issues: +#2653266: Avoid excess t() calls in field_views_field_default_views_data(), +#2653214: Replace views_include() with include_once, +#2760419: get_option() micro optimization

These other performance fixes all play nice together. I recommend all of them for 7.x-3.17 please and thanks.

dsutter’s picture

dsutter commented

RTBC+ patch #2

gdaw’s picture

gdaw commented

Nice performance boost thanks joelpittet, we have been using these for some time now.

RTBC for patch #2

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Parent issue: » #2903944: Plan for Views 7.x-3.19 release

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2: reduce_the_calls_to_the-2760405-2.patch, failed testing. View results

damienmckenna’s picture

damienmckenna
Location TN, USA
commented

Could someone please reroll this? Thanks.

joseph.olstad’s picture

joseph.olstad
Primary language French
commented
Status: Needs work » Needs review
StatusFileSize
new reduce_the_calls_to_the-2760405-12.patch2.52 KB

rerolled for 7.x-3.x latest

joseph.olstad’s picture

joseph.olstad
Primary language French
commented
Status: Needs review » Reviewed & tested by the community

it was only a whitespace difference that caused the patch to fail.
returning to RTBC because it's the exact same code, except without the broken empty line.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Reviewed & tested by the community » Fixed

Thanks! Committed.

Status: Fixed » Closed (fixed)

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