This is a bug in Drupal 7 and Drupal 8.

  $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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

catch’s picture

Status: Active » Needs review
FileSize
1.79 KB

Here's the backwards-compatible version.

plach’s picture

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.

Dries’s picture

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

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

David_Rothstein’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
988 bytes

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"?

plach’s picture

Status: Needs review » Reviewed & tested by the community

Looks good.

webchick’s picture

Priority: Major » Normal

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

David_Rothstein’s picture

Status: Reviewed & tested by the community » Fixed

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