The function backup_migrate_help initializes the array with help strings before checking whether it is needed. This leads to a minor performance hit on practically each page request as the translate function is called multiple times (database lookup per call).

Possible solutions:
- use a switch statement
- bail out early if $section does not start with BACKUP_MIGRATE_MENU_PATH or is not equal to admin/help#backup_migrate (this solution is, looking at the logic regarding admin/help#backup_migrate, probably the simpler one but will still call too many t()'s on your own pages.

CommentFileSizeAuthor
#8 backup_migrate-n1042672-7.patch11.75 KBBrankoC
#2 1042672.patch10.08 KBfietserwin
#1 1042672.patch10.08 KBfietserwin
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

fietserwin’s picture

fietserwin
Location Le Mont-Dore
CreditAttribution: fietserwin commented
FileSize
1042672.patch10.08 KB

Please find attached a patch that solves the problem as well as serves advanced help.

fietserwin’s picture

fietserwin
Location Le Mont-Dore
CreditAttribution: fietserwin commented
FileSize
1042672.patch10.08 KB

Oops, typo in patch. Please use this one.

fietserwin’s picture

fietserwin
Location Le Mont-Dore
CreditAttribution: fietserwin commented
Status: Active » Needs review
fietserwin’s picture

fietserwin
Location Le Mont-Dore
CreditAttribution: fietserwin commented
Status: Needs review » Reviewed & tested by the community

Simple patch, hardly anybody will notice this problem, so few chances to get reviewed by other people. So, I push this one to RTBC.

couturier’s picture

couturier CreditAttribution: couturier as a volunteer commented
Version: 7.x-2.x-dev » 7.x-3.x-dev
Issue summary: View changes
Status: Reviewed & tested by the community » Needs work

The 7.x-2.x branch will be deprecated soon. See this issue: Verify 7.x-2.x -to- 7.x-3.x upgrade path, mark 7.x-2.x as unsupported.

If this issue still exists in the 7.x-3.x branch, the patch will need to be rewritten.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 2: 1042672.patch, failed testing. View results

BrankoC’s picture

BrankoC CreditAttribution: BrankoC as a volunteer commented
Status: Needs work » Needs review
FileSize
backup_migrate-n1042672-7.patch11.75 KB

Reroll against 7.x-3.x.

I am not sure how useful this is, as the increased page load time of the initial situation seems minimal, but I guess it is never a bad idea to be a good citizen.

1) I manually applied #2.

2) Fixed some minor (but perhaps not all) coding standards issues.

3) Added in the access checks that had been added to backup_migrate_help() since patch #2 was released.

DamienMcKenna’s picture

DamienMcKenna
Location NH, USA
CreditAttribution: DamienMcKenna at Mediacurrent commented
Status: Needs review » Fixed
Parent issue: » #3020775: Plan for Backup and Migrate 7.x-3.7

Committed. Thanks everyone.

Status: Fixed » Closed (fixed)

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