This is a bug in Drupal 7 and Drupal 8.

<?php
  $variables
['created'] = format_date($comment->created);
 
$variables['changed'] = format_date($comment->changed);
?>

The template provided by comment module renders neither of these, but format_date() is very expensive when called up to 600 times per page.

One obvious fix which should be backportable to Drupal 7 would be to only format the date once if $comment->change and $comment->created are the same timestamp.

For Drupal 8 we should look at whether Twig will let us take these out of generic preprocess and lazy-generate the variables based on the compiled template (i.e. it's supposed to know whether specific variables are used or not, so there shouldn't be a need to front-load things that are never printed). If not, let's just stop preparing those variables and let people use their own preprocess in custom themes if they want to.

Files: 
CommentFileSizeAuthor
#4 comment-1857956-4.patch988 bytesDavid_Rothstein
PASSED: [[SimpleTest]]: [MySQL] 39,718 pass(es).
[ View ]
#1 comment_1857956.patch1.79 KBcatch
PASSED: [[SimpleTest]]: [MySQL] 48,909 pass(es).
[ View ]

Comments

Status:Active» Needs review
StatusFileSize
new1.79 KB
PASSED: [[SimpleTest]]: [MySQL] 48,909 pass(es).
[ View ]

Here's the backwards-compatible version.

Status:Needs review» Reviewed & tested by the community

Looks sensible to me as a stop-gap fix, altough I'd prefer a ternary here :)

I don't think this needs tests.

Version:8.x-dev» 7.x-dev

Makes sense. Committed to 8.x. Moving to 7.x for Angie to commit.

Status:Reviewed & tested by the community» Needs review
StatusFileSize
new988 bytes
PASSED: [[SimpleTest]]: [MySQL] 39,718 pass(es).
[ View ]

Looks reasonable to me, but the patch didn't remotely apply to Drupal 7. Rerolled, but it looks like only half of it may actually be relevant in D7.

Is this really a big enough performance improvement to be considered "major"?

Status:Needs review» Reviewed & tested by the community

Looks good.

Priority:Major» Normal

In D7, no, I don't think so. It's a much bigger deal in D8 with CMI.

Status:Reviewed & tested by the community» Fixed

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