From 7ff211065555525d9c934a3209dcecbc50bbf74a Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 26 Feb 2014 13:17:28 -0800
Subject: [PATCH] Stop markup cache from hitting cache O(N^2) times

Summary:
The intent of getOrBuildEngine() is to save some boilerplate in cases where we're just using a standard engine, but it didn't get cached so we'd rebuilt it over and over again.

This was especially bad in Differential with a large number of inlines.

Test Plan: "Query" tab of services is no longer quite so ridiculous in Differential.

Reviewers: btrahan, dctrwatson

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D8352
---
 .../DifferentialRevisionViewController.php    |  3 --
 .../PhabricatorApplicationTransactionView.php | 28 +++++++++----------
 2 files changed, 14 insertions(+), 17 deletions(-)

diff --git a/src/applications/differential/controller/DifferentialRevisionViewController.php b/src/applications/differential/controller/DifferentialRevisionViewController.php
index 2bbcfd1ecb..23cfa1b906 100644
--- a/src/applications/differential/controller/DifferentialRevisionViewController.php
+++ b/src/applications/differential/controller/DifferentialRevisionViewController.php
@@ -903,9 +903,6 @@ final class DifferentialRevisionViewController extends DifferentialController {
       ->needComments(true)
       ->execute();
 
-    $engine = id(new PhabricatorMarkupEngine())
-      ->setViewer($viewer);
-
     $timeline = id(new DifferentialTransactionView())
       ->setUser($viewer)
       ->setObjectPHID($revision->getPHID())
diff --git a/src/applications/transactions/view/PhabricatorApplicationTransactionView.php b/src/applications/transactions/view/PhabricatorApplicationTransactionView.php
index b02c6119ad..cc04b5512d 100644
--- a/src/applications/transactions/view/PhabricatorApplicationTransactionView.php
+++ b/src/applications/transactions/view/PhabricatorApplicationTransactionView.php
@@ -149,23 +149,23 @@ class PhabricatorApplicationTransactionView extends AphrontView {
   }
 
   protected function getOrBuildEngine() {
-    if ($this->engine) {
-      return $this->engine;
-    }
+    if (!$this->engine) {
+      $field = PhabricatorApplicationTransactionComment::MARKUP_FIELD_COMMENT;
 
-    $field = PhabricatorApplicationTransactionComment::MARKUP_FIELD_COMMENT;
-
-    $engine = id(new PhabricatorMarkupEngine())
-      ->setViewer($this->getUser());
-    foreach ($this->transactions as $xaction) {
-      if (!$xaction->hasComment()) {
-        continue;
+      $engine = id(new PhabricatorMarkupEngine())
+        ->setViewer($this->getUser());
+      foreach ($this->transactions as $xaction) {
+        if (!$xaction->hasComment()) {
+          continue;
+        }
+        $engine->addObject($xaction->getComment(), $field);
       }
-      $engine->addObject($xaction->getComment(), $field);
-    }
-    $engine->process();
+      $engine->process();
 
-    return $engine;
+      $this->engine = $engine;
+    }
+
+    return $this->engine;
   }
 
   private function buildChangeDetailsLink(