From 9efcbc4ee95878e51ca381039f03038ea8ca9c28 Mon Sep 17 00:00:00 2001
From: Joel Beales <joelb@fb.com>
Date: Tue, 19 Nov 2013 12:06:24 -0800
Subject: [PATCH] Speed up loading of diffs with a lot of unit test failures

Summary:
We've been having trouble with viewing diffs timing out when there's a lot of unit test failures. It was caused by formatting userdata for every single failure. The expensive part of this was actually creating the engine for every result, so moved the construction outside of the loop.

Diffs that timed out (2 min) loading before load in around 6 seconds now.

Test Plan: Loaded diffs that used to time out. Verified that details still looked right when Show Full Unit Test Results Is Clicked.

Reviewers: epriestley, keegancsmith, lifeihuang, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran, andrewjcg

Differential Revision: https://secure.phabricator.com/D7581
---
 .../DifferentialUnitFieldSpecification.php    | 31 ++++++++++++-------
 .../markup/PhabricatorMarkupEngine.php        |  4 ++-
 2 files changed, 23 insertions(+), 12 deletions(-)

diff --git a/src/applications/differential/field/specification/DifferentialUnitFieldSpecification.php b/src/applications/differential/field/specification/DifferentialUnitFieldSpecification.php
index 546dcd2177..a3d701475e 100644
--- a/src/applications/differential/field/specification/DifferentialUnitFieldSpecification.php
+++ b/src/applications/differential/field/specification/DifferentialUnitFieldSpecification.php
@@ -70,8 +70,24 @@ final class DifferentialUnitFieldSpecification
         $udata[$key]['sort'] = idx($sort_map, idx($test, 'result'));
       }
       $udata = isort($udata, 'sort');
-
-      foreach ($udata as $test) {
+      $engine = new PhabricatorMarkupEngine();
+      $engine->setViewer($this->getUser());
+      $markup_objects = array();
+      foreach ($udata as $key => $test) {
+        $userdata = idx($test, 'userdata');
+        if ($userdata) {
+          if ($userdata !== false) {
+            $userdata = str_replace("\000", '', $userdata);
+          }
+          $markup_object = id(new PhabricatorMarkupOneOff())
+              ->setContent($userdata)
+              ->setPreserveLinebreaks(true);
+          $engine->addObject($markup_object, 'default');
+          $markup_objects[$key] = $markup_object;
+        }
+      }
+      $engine->process();
+      foreach ($udata as $key => $test) {
         $result = idx($test, 'result');
 
         $default_hide = false;
@@ -110,17 +126,10 @@ final class DifferentialUnitFieldSpecification
           'show'  => $show,
         );
 
-        $userdata = idx($test, 'userdata');
-        if ($userdata) {
-          if ($userdata !== false) {
-            $userdata = str_replace("\000", '', $userdata);
-          }
-          $engine = PhabricatorMarkupEngine::newDifferentialMarkupEngine();
-          $engine->setConfig('viewer', $this->getUser());
-          $userdata = $engine->markupText($userdata);
+        if (isset($markup_objects[$key])) {
           $rows[] = array(
             'style' => 'details',
-            'value' => $userdata,
+            'value' => $engine->getOutput($markup_objects[$key], 'default'),
             'show'  => false,
           );
           if (empty($hidden['details'])) {
diff --git a/src/infrastructure/markup/PhabricatorMarkupEngine.php b/src/infrastructure/markup/PhabricatorMarkupEngine.php
index 764ca6f79a..ff01cb4bc1 100644
--- a/src/infrastructure/markup/PhabricatorMarkupEngine.php
+++ b/src/infrastructure/markup/PhabricatorMarkupEngine.php
@@ -242,7 +242,9 @@ final class PhabricatorMarkupEngine {
     }
 
     foreach ($objects as $key => $info) {
-      if (isset($blocks[$key])) {
+      // False check in case MySQL doesn't support unicode characters
+      // in the string (T1191), resulting in unserialize returning false.
+      if (isset($blocks[$key]) && $blocks[$key]->getCacheData() !== false) {
         // If we already have a preprocessing cache, we don't need to rebuild
         // it.
         continue;
-- 
2.54.0