Hi,
After running a XHProf across a site I seen that there were a large number of calls to t(). After digging into the function, it seems all calls are running through the translation even if the language options are not really even leveraged. On the platform which I am currently maintaining, there isn't really a need for language options so it would be good if we didn't have to run through it if it's not required - I can however see benefit in keeping this for multilingual sites.

My proposal is to return early should the $options array contain a 'skip_lang' key set to TRUE. After a bit more profiling, it appears to make a small difference which adds up over time (results using 10000 iterations on a fresh Drupal 7 site).

Current

IDENTIFIER       EXECUTION TIME
t()              0.12099814ms
t() with args    0.23800611ms

IDENTIFIER             EXECUTION TIME
t()                    0.11307693ms
t() with args          0.22072194ms
t() using skip_lang    0.10611510ms
CommentFileSizeAuthor
#1 allow-skip-translation-in-t-2412675-1.patch1.08 KBjacobbednarz
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jacobbednarz’s picture

jacobbednarz CreditAttribution: jacobbednarz commented
Issue summary: View changes
FileSize
allow-skip-translation-in-t-2412675-1.patch1.08 KB

Attaching a patch with the proposed updates.

jacobbednarz’s picture

jacobbednarz CreditAttribution: jacobbednarz commented

Along with this, I am also questioning whether you need to do checks like return (empty($args)) ? $string : format_string($string, $args); as format_string() does essentially this on it's own.

jacobbednarz’s picture

jacobbednarz CreditAttribution: jacobbednarz commented
Status: Active » Needs review
jacobbednarz’s picture

jacobbednarz CreditAttribution: jacobbednarz commented
Issue summary: View changes
joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
Issue tags: -t()

How is that option to be set and what is the use-case? I could see this as a global variable or hinging on one language or something but an option to pass in to the t() sounds not very practical.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
Issue tags: +i18n
jacobbednarz’s picture

jacobbednarz CreditAttribution: jacobbednarz commented

Thanks for following up Joel.

I've since left the team and company that I initially proposed this for so I'm not in a rush to get this one patched however it might be worth while shipping to give larger (non multi-lingual) sites a bit of a boost. Looking back on this patch, it might be more appropriate to do something like the following:

if (!empty($conf['skip_lang']) && $conf['skip_lang'] == TRUE) {
  return (empty($args)) ? $string : format_string($string, $args);
}

In this case, it would instead rely on a $conf['skip_lang'] which could be set at a site configuration wide level instead. Thoughts? I'm happy to put up patch for this if you still think it's worth while.

ndobromirov’s picture

ndobromirov CreditAttribution: ndobromirov commented

Another micro-optimization - the condition:
if (isset($options['skip_lang']) && $options['skip_lang'] == TRUE)

Can be written as:
if (isset($options['skip_lang']) && $options['skip_lang'])

Sparing the comparison calculation.

Nit - I personally do not like the code duplication of the following logic:
return (empty($args)) ? $string : format_string($string, $args);

stefan.r’s picture

stefan.r CreditAttribution: stefan.r commented

Wouldn't this apply to 8.x as well?

jacobbednarz’s picture

jacobbednarz CreditAttribution: jacobbednarz commented

@stefan.r: perhaps but as I've mentioned, I since left and am no longer looking into this (or D8). I'm happy to do the work to get this over the line if someone from core thinks this is valuable as there are performance wins to be had however I won't be porting it to D8 unless this lands.