Problem/Motivation

addressfield_get_administrative_areas() calls t() nearly 1000 times in one call.
On the shipping page I noticed it this function was called only 8 times, but it was amounting to 80ms wall time.

Proposed resolution

Statically save the first set.

Remaining tasks

RTBC & Commit

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

bojanz’s picture

I'd also argue that most administrative areas shouldn't be passed through t() in the first place.

joelpittet’s picture

Status: Active » Needs review
FileSize
131.92 KB
146.87 KB
2.45 KB

@bojanz That may take more thought into which should or shouldn't use t(). Feel free to take this issue and run with it.

I was also thinking, why is it getting them all, why not only return ones for the country requested?

Before

After

SurajHo’s picture

Nice job on the patch. I went from almost 180000 calls to a little under 47000. It would be nice if this patch would be added to the next release although I'm also wondering why doesn't just return the countries that are needed.

bojanz’s picture

Status: Needs review » Needs work

I've now significantly reduced our usage of t(): http://cgit.drupalcode.org/addressfield/commit/?id=2ffb831
(following #1 and a discussion on IRC with joelpittet).

A static and/or a refactoring of how the data is stored is the next step.
Marking as "needs work" because the patch won't apply anymore, but feel free to leave it to me.

joelpittet’s picture

Status: Needs work » Needs review
FileSize
1.79 KB

I'm going to leave the additional t() for JM out of this patch because I'm sure if they are needed we can open a new issue.

joelpittet’s picture

Issue summary: View changes
bojanz’s picture

Status: Needs review » Fixed

Moved the static as discussed, and documented it. Thanks!

  • bojanz committed c8a4d1f on 7.x-1.x authored by joelpittet
    Issue #2507665 by joelpittet: addressfield_get_administrative_areas...

Status: Fixed » Closed (fixed)

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