Commits

Wez Furlong  committed 5523754

make an effort to improve timeline load performance

Turns out that the cache keys weren't matching for object arguments;
changed to using json_encode so that we have a consistent
representation.

This uncovered a strong-caching problem with hg based wikis due to the
overzealous attempt to cache the "log" output in the mercurial code.
Removed that cache attempt.

Break the cache files up into a three-level directory structure to avoid
directory bloat.

Made schema tool blow the whole cache on upgrades. This is a little
heavy handed but feels safest for overall consistency.

Decompose the timeline fetching and rendering code and cache the
individual renders. This gives a better balance for showing new items
sooner and not having to work so hard to show the older items.

  • Participants
  • Parent commits d38a535

Comments (0)

Files changed (5)

File bin/schema-tool.php

 $q->execute(array($latest->version));
 $db->commit();
 
+mtrack_cache_blow_all();
 
+

File inc/cache.php

 <?php # vim:ts=2:sw=2:et:
 /* For licensing and copyright terms, see the file named LICENSE */
 
+function mtrack_cache_maintain_file($filename, $max_cache_life)
+{
+  $st = stat($filename);
+  if ($st['mtime'] + $max_cache_life < time()) {
+    unlink($filename);
+  }
+}
+
+function mtrack_cache_maintain_dir($cachedir, $max_cache_life)
+{
+  foreach (scandir($cachedir) as $name) {
+    if ($name[0] == '.') continue;
+    $filename = "$cachedir/$name";
+    if (is_file($filename)) {
+      mtrack_cache_maintain_file($filename, $max_cache_life);
+    } else {
+      mtrack_cache_maintain_dir($filename, $max_cache_life);
+    }
+  }
+}
+
 /* maintain cache */
 function mtrack_cache_maintain($max_cache_life = null)
 {
       $max_cache_life = 14 * 86400;
     }
   }
-  foreach (scandir($cachedir) as $name) {
-    $filename = "$cachedir/$name";
-    if (!is_file($filename)) {
-      continue;
-    }
-    $st = stat($filename);
-    if ($st['mtime'] + $max_cache_life < time()) {
-      unlink($filename);
-    }
-  }
+  mtrack_cache_maintain_dir($cachedir);
 }
 
 function mtrack_cache_blow_all()
 {
   $cachedir = MTrackConfig::get('core', 'vardir') . '/cmdcache';
   foreach (scandir($cachedir) as $name) {
+    if ($name[0] == '.') continue;
     $filename = "$cachedir/$name";
     if (is_file($filename)) {
       unlink($filename);
-    }
-  }
-}
-
-/* walks the cache; loads each element and examines the keys.
- * if the key prefix matches $key, that element is removed */
-function mtrack_cache_blow_matching($key)
-{
-  $cachedir = MTrackConfig::get('core', 'vardir') . '/cmdcache';
-  foreach (scandir($cachedir) as $name) {
-    $filename = "$cachedir/$name";
-    if (!is_file($filename)) {
-      continue;
-    }
-    $fp = @fopen($filename, 'r');
-    if (!$fp) {
-      continue;
-    }
-    flock($fp, LOCK_SH);
-    $data = unserialize(stream_get_contents($fp));
-    flock($fp, LOCK_UN);
-    $fp = null;
-
-    $match = true;
-    foreach ($key as $i => $element) {
-      if ($data->key[$i] != $element) {
-        $match = false;
-        break;
-      }
-    }
-    if ($match) {
-      unlink("$cachedir/$name");
+    } else {
+      mtrack_rmdir($filename);
     }
   }
 }
 function mtrack_cache_key($func, $args, $key = null)
 {
   if ($key === null) {
-    $fkey = var_export($args, true);
+    $fkey = json_encode($args);
     $key = $fkey;
   } else {
-    $fkey = var_export($key, true);
+    $fkey = json_encode($key);
   }
   if (is_string($func)) {
     $fkey = "$func$fkey";
   } else {
-    $fkey = var_export($func, true) . $fkey;
+    $fkey = json_encode($func) . $fkey;
   }
 
   $cachedir = MTrackConfig::get('core', 'vardir') . '/cmdcache';
-  $cachefile = $cachedir . '/' .  sha1($fkey);
+  $hash = sha1($fkey);
+  /* make three levels to avoid creating huge directories */
+  $a = substr($hash, 0, 2);
+  $b = substr($hash, 2, 2);
+  $c = substr($hash, 4);
+  $cachefile = "$cachedir/$a/$b/$c";
 
   return array($key, $fkey, $cachefile);
 }
 {
   list($key, $fkey, $cachefile) = mtrack_cache_key($func, $args, $key);
 
+#  error_log("blow: $fkey $cachefile");
   if (file_exists($cachefile)) {
     unlink($cachefile);
   }
 
   list($key, $fkey, $cachefile) = mtrack_cache_key($func, $args, $key);
 
+  mtrack_mkdir_p(dirname($cachefile));
+#  error_log("cache: $fkey $cachefile");
+
   $updating = false;
   for ($i = 0; $i < 10; $i++) {
     $fp = @fopen($cachefile, 'r+');

File inc/scm/hg.php

     }
     $args[] = $path;
 
-    return mtrack_cache(
-      array($this, '_parse_log'),
-      array($args),
-      30);
+    return $this->_parse_log($args);
   }
 
 

File inc/timeline.php

   }
 }
 
+function mtrack_render_timeline_item($d, $row)
+{
+  global $ABSWEB;
+
+  $time = $d->format('H:i');
+  $day = $d->format('D, M d Y');
+
+  // figure out an event type based on the object and the reason
+  if (strpos($row['object'], ':') !== false) {
+    list($object, $id) = explode(':', $row['object'], 3);
+  } else {
+    $id = 0;
+    $object = $row['object'];
+  }
+  $eventclass = '';
+  $item = $row['object'];
+  switch ($object) {
+  case 'ticket':
+    if (!strncmp($row['reason'], 'created ', 8)) {
+      $eventclass = ' newticket';
+    } elseif (!strncmp($row['reason'], 'closed ', 7)) {
+      $eventclass = ' closedticket';
+    } else {
+      $eventclass = ' editticket';
+    }
+    $item = "Ticket " . mtrack_ticket($id);
+    if (MTrackConfig::get('core', 'wikisyntax') == 'markdown') {
+      /* need a blank line to successfully start a list */
+      $row['reason'] .= "\n";
+    }
+    foreach ($row['audit'] as $audit) {
+      if (!preg_match("/^ticket:$id:@?(.*)$/", $audit['fieldname'], $M)) {
+        continue;
+      }
+      $fieldname = $M[1];
+      if ($fieldname == 'comment' || $fieldname == 'nsident') {
+        continue;
+      }
+      $value = $audit['value'];
+      switch ($fieldname) {
+      case 'ptid':
+        $value = strlen($value) ? "[ticket:$value]" : "deleted";
+        $fieldname = "Parent";
+        break;
+      case 'dependencies':
+      case 'blocks':
+      case 'children':
+        $value = array();
+        foreach (explode(',', $audit['value']) as $t) {
+          $value[] = "[ticket:$t]";
+        }
+        $value = join(" ", $value);
+        break;
+      case 'milestones':
+        $value = array();
+        foreach (explode(',', $audit['value']) as $t) {
+          $value[] = "[milestone:$t]";
+        }
+        $value = join(" ", $value);
+        break;
+      case 'keywords':
+        $value = array();
+        foreach (explode(',', $audit['value']) as $t) {
+          $value[] = "[keyword:$t]";
+        }
+        $value = join(" ", $value);
+        break;
+      case 'components':
+        $value = array();
+        foreach (explode(',', $audit['value']) as $t) {
+          $value[] = "[component:$t]";
+        }
+        $value = join(" ", $value);
+        break;
+      }
+      $f = MTrackTicket_CustomFields::getInstance()
+        ->fieldByName($fieldname);
+      if ($f) {
+        $fieldname = $f->label;
+      } else {
+        $fieldname = ucfirst($fieldname);
+      }
+      $row['reason'] .= "\n * $fieldname -> $value";
+    }
+    $row['reason'] .= "\n";
+    break;
+
+  case 'wiki':
+    /* we ignore these; they're were created by the wiki UI,
+     * but have been superseded by the repo entry instead */
+    return null;
+
+  case 'milestone':
+    $eventclass = ' editmilestone';
+    $item = mtrack_object_id_link('milestone', $id);
+    break;
+  case 'changeset':
+    /* these are only present in installations that were migrated
+     * from trac */
+    $eventclass = ' newchangeset';
+    preg_match("/^changeset:(.*):([^:]+)$/", $row['object'], $M);
+    $repo = $M[1];
+    if (!_mtrack_timeline_is_repo_visible($repo)) {
+      return null;
+    }
+    $id = $M[2];
+    $item = "<a href='{$ABSWEB}browse.php/$repo'>$repo</a> change " .
+      mtrack_changeset($id, $repo);
+    break;
+  case 'snippet':
+    $item = "<a href='{$ABSWEB}snippet.php/$id'>View Snippet</a>";
+    break;
+  case 'repo':
+    static $repos = array();
+    if (!_mtrack_timeline_is_repo_visible($id)) {
+      return null;
+    }
+    if (!isset($repos[$id])) {
+      $repos[$id] = MTrackRepo::loadById($id);
+    }
+    if (is_object($repos[$id])) {
+      $R = $repos[$id];
+      $name = MTrackRepo::makeDisplayName($R);
+      $item = "<a href='{$ABSWEB}browse.php/$name'>$name</a>";
+      /* pre-existing installations may not have any audit entries */
+      if (!isset($row['audit'])) {
+        $row['audit'] = array();
+      }
+      if ($name == 'default/wiki') {
+        /* pull out the list of modified files from the change audit */
+        $pages = array();
+        $suf = MTrackConfig::get('core', 'wikifilenamesuffix');
+        foreach ($row['audit'] as $audit) {
+          if (!preg_match("/^repo:\d+:rev:/", $audit['fieldname'])) {
+            continue;
+          }
+          $ent = json_decode($audit['value']);
+          /* if a suffix is defined, only include pages that have the
+           * suffix, otherwise include any pages. We remove the suffix from
+           * the names of pages that we return */
+          foreach ($ent->files as $page) {
+            if ($suf) {
+              if (substr($page, -strlen($suf)) == $suf) {
+                $pages[$page] =
+                  substr($page, 0, strlen($page) - strlen($suf));
+              }
+            } else {
+              $pages[$page] = $page;
+            }
+          }
+        }
+        if (count($pages)) {
+          $item = '';
+          foreach ($pages as $page) {
+            $item .= ' ' . mtrack_wiki($page);
+          }
+        }
+      } else {
+        /* not wiki.  This is a placeholder for changeset(s) in a
+         * changegroup.  If so, those may reference a ticket.  If not, then
+         * we want to emit them now */
+        $checker = new MTrackCommitChecker($R);
+        foreach ($row['audit'] as $audit) {
+          if (!preg_match("/^repo:\d+:rev:/", $audit['fieldname'])) {
+            continue;
+          }
+          $ent = json_decode($audit['value']);
+          $a = $checker->parseCommitMessage($ent->changelog);
+          if (!count($a)) {
+            /* doesn't ref a ticket; ensure that we see which
+             * changeset it came from if it doesn't already
+             * mention it */
+
+            if ($ent->rev === null) {
+              // Ugh, workaround a bug that recorded the rev
+              // as null in the audit.
+              continue;
+            }
+
+            $cslink =
+              "[changeset:" . $R->getBrowseRootName() . ',' .
+              $ent->rev . ']';
+            if (strpos($row['reason'], $cslink) === false) {
+              $row['reason'] .= " (In $cslink)";
+            }
+            if (strpos($row['reason'], trim($ent->changelog)) === false) {
+              $row['reason'] .= ' ' . $ent->changelog;
+            }
+          }
+        }
+        if (!strlen(trim($row['reason']))) {
+          /* if there's nothing to say about the change, don't show it
+           * in the timeline */
+          return null;
+        }
+      }
+    } else {
+      $item = "&lt;item has been deleted&gt;";
+    }
+    break;
+  }
+
+  $reason = MTrackWiki::format_to_oneliner($row['reason'], 12);
+
+  $html = "<div class='timelineevent'>" .
+    mtrack_username($row['who'], array(
+      'no_name' => true,
+      'size' => 48,
+      'class' => 'timelineface'
+    )) .
+    "<div class='timelinetext'>" .
+    "<div class='timelinereason'>" .
+    "$reason</div>\n" .
+    "<span class='time'>$time</span> $item by " .
+    mtrack_username($row['who'], array('no_image' => true)) .
+    "</div>\n" .
+    "</div>\n";
+
+  return array($day, $row, $html);
+}
+
 function mtrack_render_timeline($user = null)
 {
   global $ABSWEB;
 
   $limit = 500;
-  $events = mtrack_cache('mtrack_get_timeline',
-    array('-6 weeks', $user, $limit), 60, array('Timeline', $user));
+
+  /* get the newest items first with a short ttl */
+  $newest = mtrack_cache('mtrack_get_timeline',
+    array('-2 minutes', $user, $limit), 5);
+
+  $older = mtrack_cache('mtrack_get_timeline',
+    array('-6 weeks', $user, $limit), 60);
+
+  $events = array_merge($newest, $older);
 
   echo "<div class='timeline'>";
-  $last_date = null;
+  $items = array();
+  $last_row = null;
+
   foreach ($events as $event_index => $row) {
-    if (--$limit == 0) {
+    if (count($items) >= $limit) {
       break;
     }
 
+    if (isset($items[$row['cid']])) {
+      /* already have this one; overlap from the caches */
+      continue;
+    }
+
+    /* avoid spam */
+    if ($last_row && $row['reason'] == $last_row['reason'] &&
+        $last_row['who'] == $row['who'] &&
+        $last_row['object'] == $row['object'])
+    {
+      continue;
+    }
+
     $d = date_create($row['changedate'], new DateTimeZone('UTC'));
     date_timezone_set($d, new DateTimeZone(date_default_timezone_get()));
-    $time = $d->format('H:i');
-    $day = $d->format('D, M d Y');
 
+    $item = mtrack_cache('mtrack_render_timeline_item',
+      array($d, $row), 86400);
+
+    if (!$item) {
+      continue;
+    }
+
+    $items[$row['cid']] = $item;
+    $last_row = $row;
+  }
+  $last_date = null;
+  foreach ($items as $item) {
+    list($day, $row, $html) = $item;
     if ($last_date != $day) {
       echo "<h1 class='timelineday'>$day</h1>\n";
       $last_date = $day;
     }
+    echo $html;
+  }
 
-    // figure out an event type based on the object and the reason
-    if (strpos($row['object'], ':') !== false) {
-      list($object, $id) = explode(':', $row['object'], 3);
-    } else {
-      $id = 0;
-      $object = $row['object'];
-    }
-    $eventclass = '';
-    $item = $row['object'];
-    switch ($object) {
-      case 'ticket':
-        if (!strncmp($row['reason'], 'created ', 8)) {
-          $eventclass = ' newticket';
-        } elseif (!strncmp($row['reason'], 'closed ', 7)) {
-          $eventclass = ' closedticket';
-        } else {
-          $eventclass = ' editticket';
-        }
-        $item = "Ticket " . mtrack_ticket($id);
-        if (MTrackConfig::get('core', 'wikisyntax') == 'markdown') {
-          /* need a blank line to successfully start a list */
-          $row['reason'] .= "\n";
-        }
-        foreach ($row['audit'] as $audit) {
-          if (!preg_match("/^ticket:$id:@?(.*)$/", $audit['fieldname'], $M)) {
-            continue;
-          }
-          $fieldname = $M[1];
-          if ($fieldname == 'comment' || $fieldname == 'nsident') {
-            continue;
-          }
-          $value = $audit['value'];
-          switch ($fieldname) {
-          case 'ptid':
-              $value = strlen($value) ? "[ticket:$value]" : "deleted";
-              $fieldname = "Parent";
-              break;
-            case 'dependencies':
-            case 'blocks':
-            case 'children':
-              $value = array();
-              foreach (explode(',', $audit['value']) as $t) {
-                $value[] = "[ticket:$t]";
-              }
-              $value = join(" ", $value);
-              break;
-            case 'milestones':
-              $value = array();
-              foreach (explode(',', $audit['value']) as $t) {
-                $value[] = "[milestone:$t]";
-              }
-              $value = join(" ", $value);
-              break;
-            case 'keywords':
-              $value = array();
-              foreach (explode(',', $audit['value']) as $t) {
-                $value[] = "[keyword:$t]";
-              }
-              $value = join(" ", $value);
-              break;
-            case 'components':
-              $value = array();
-              foreach (explode(',', $audit['value']) as $t) {
-                $value[] = "[component:$t]";
-              }
-              $value = join(" ", $value);
-              break;
-          }
-          $f = MTrackTicket_CustomFields::getInstance()
-            ->fieldByName($fieldname);
-          if ($f) {
-            $fieldname = $f->label;
-          } else {
-            $fieldname = ucfirst($fieldname);
-          }
-          $row['reason'] .= "\n * $fieldname -> $value";
-        }
-        $row['reason'] .= "\n";
-        break;
-      case 'wiki':
-        /* we ignore these; they're were created by the wiki UI,
-         * but have been superseded by the repo entry instead */
-        continue 2;
-      case 'milestone':
-        if ($event_index > 0) {
-          /* Planning screen generates a lot of noisy entries */
-          $prior = $events[$event_index-1];
-          if ($prior['object'] == "milestone:$id" &&
-              $prior['reason'] == $row['reason']) {
-            // Don't count it against the limit
-            $limit++;
-            continue 2;
-          }
-        }
-        $eventclass = ' editmilestone';
-        $item = mtrack_object_id_link('milestone', $id);
-        break;
-      case 'changeset':
-        /* these are only present in installations that were migrated
-         * from trac */
-        $eventclass = ' newchangeset';
-        preg_match("/^changeset:(.*):([^:]+)$/", $row['object'], $M);
-        $repo = $M[1];
-        if (!_mtrack_timeline_is_repo_visible($repo)) {
-          continue 2;
-        }
-        $id = $M[2];
-        $item = "<a href='{$ABSWEB}browse.php/$repo'>$repo</a> change " .
-                mtrack_changeset($id, $repo);
-        break;
-      case 'snippet':
-        $item = "<a href='{$ABSWEB}snippet.php/$id'>View Snippet</a>";
-        break;
-      case 'repo':
-        static $repos = array();
-        if (!_mtrack_timeline_is_repo_visible($id)) {
-          continue 2;
-        }
-        if (!isset($repos[$id])) {
-          $repos[$id] = MTrackRepo::loadById($id);
-        }
-        if (is_object($repos[$id])) {
-          $R = $repos[$id];
-          $name = MTrackRepo::makeDisplayName($R);
-          $item = "<a href='{$ABSWEB}browse.php/$name'>$name</a>";
-          /* pre-existing installations may not have any audit entries */
-          if (!isset($row['audit'])) {
-            $row['audit'] = array();
-          }
-          if ($name == 'default/wiki') {
-            /* pull out the list of modified files from the change audit */
-            $pages = array();
-            $suf = MTrackConfig::get('core', 'wikifilenamesuffix');
-            foreach ($row['audit'] as $audit) {
-              if (!preg_match("/^repo:\d+:rev:/", $audit['fieldname'])) {
-                continue;
-              }
-              $ent = json_decode($audit['value']);
-              /* if a suffix is defined, only include pages that have the
-               * suffix, otherwise include any pages. We remove the suffix from
-               * the names of pages that we return */
-              foreach ($ent->files as $page) {
-                if ($suf) {
-                  if (substr($page, -strlen($suf)) == $suf) {
-                    $pages[$page] =
-                      substr($page, 0, strlen($page) - strlen($suf));
-                  }
-                } else {
-                  $pages[$page] = $page;
-                }
-              }
-            }
-            if (count($pages)) {
-              $item = '';
-              foreach ($pages as $page) {
-                $item .= ' ' . mtrack_wiki($page);
-              }
-            }
-          } else {
-            /* not wiki.  This is a placeholder for changeset(s) in a
-             * changegroup.  If so, those may reference a ticket.  If not, then
-             * we want to emit them now */
-            $checker = new MTrackCommitChecker($R);
-            foreach ($row['audit'] as $audit) {
-              if (!preg_match("/^repo:\d+:rev:/", $audit['fieldname'])) {
-                continue;
-              }
-              $ent = json_decode($audit['value']);
-              $a = $checker->parseCommitMessage($ent->changelog);
-              if (!count($a)) {
-                /* doesn't ref a ticket; ensure that we see which
-                 * changeset it came from if it doesn't already
-                 * mention it */
-
-                if ($ent->rev === null) {
-                  // Ugh, workaround a bug that recorded the rev
-                  // as null in the audit.
-                  continue;
-                }
-
-                $cslink =
-                  "[changeset:" . $R->getBrowseRootName() . ',' .
-                  $ent->rev . ']';
-                if (strpos($row['reason'], $cslink) === false) {
-                  $row['reason'] .= " (In $cslink)";
-                }
-                if (strpos($row['reason'], trim($ent->changelog)) === false) {
-                  $row['reason'] .= ' ' . $ent->changelog;
-                }
-              }
-            }
-            if (!strlen(trim($row['reason']))) {
-              /* if there's nothing to say about the change, don't show it
-               * in the timeline */
-              continue 2;
-            }
-          }
-        } else {
-          $item = "&lt;item has been deleted&gt;";
-        }
-        break;
-    }
-
-    $reason = MTrackWiki::format_to_oneliner($row['reason'], 12);
-
-    echo "<div class='timelineevent'>",
-      mtrack_username($row['who'], array(
-        'no_name' => true,
-        'size' => 48,
-        'class' => 'timelineface'
-        )),
-      "<div class='timelinetext'>",
-      "<div class='timelinereason'>",
-      "$reason</div>\n",
-      "<span class='time'>$time</span> $item by ",
-      mtrack_username($row['who'], array('no_image' => true)),
-      "</div>\n";
-    echo "</div>\n";
-  }
   echo "</div>\n";
 }
 
   return "<abbr title='$iso8601' class='timeinterval'>$full</abbr> <span class='fulldate'>$full</span>";
 }
 
+function mtrack_mkdir_p($dir)
+{
+  if (is_dir($dir)) return true;
+  $parent = dirname($dir);
+  if (!is_dir($parent)) {
+    mtrack_mkdir_p($parent);
+  }
+  return mkdir($dir);
+}
+
 function mtrack_rmdir($dir)
 {
   $files = scandir($dir);