Massive tree action speedup
Dennis Kaarsemaker [Fri, 3 Aug 2012 09:55:49 +0000 (11:55 +0200)]
The tree action used to use $tree_obj->tree to get all tree entries.
This is ridiculously slow as it loads all blobs via git cat-file.
Instead, expose directory_entries from the underlying Git::PurePerl
object and use it instead.

This also required moving _mode_str (now mode_string for consistency) to
Gitalist::Utils so it could be used in the template.

lib/Gitalist/Controller/Fragment/Ref.pm
lib/Gitalist/Controller/Root.pm
lib/Gitalist/Git/Object.pm
lib/Gitalist/Git/Object/HasTree.pm
lib/Gitalist/Utils.pm
root/fragment/ref/tree.tt2

index cbfacda..bc79659 100644 (file)
@@ -54,7 +54,7 @@ after tree => sub {
     ;
     $c->stash(
         tree      => $tree_obj,
-        tree_list => $tree_obj->tree,
+        entries   => $tree_obj->entries,
     );
 };
 
index 7b921f8..42896ea 100644 (file)
@@ -3,7 +3,7 @@ package Gitalist::Controller::Root;
 use Moose;
 use Moose::Autobox;
 use Digest::MD5 qw(md5_hex);
-use Gitalist::Utils qw/ age_string /;
+use Gitalist::Utils qw/ age_string mode_string /;
 
 use namespace::autoclean;
 
@@ -54,6 +54,9 @@ sub base : Chained('/root') PathPart('') CaptureArgs(0) {
         $uri .= "?s=$size" if $size;
         return $uri;
     },
+    mode_string => sub {
+        return mode_string(oct shift);
+    }
   );
 }
 
index 7d46632..c4e87d4 100644 (file)
@@ -6,6 +6,7 @@ class Gitalist::Git::Object with Gitalist::Git::Serializable is dirty {
 
     use MooseX::Types::Moose qw/Str Int Bool Maybe ArrayRef/;
     use MooseX::Types::Common::String qw/NonEmptySimpleStr/;
+    use Gitalist::Utils qw/mode_string/;
     use overload '""' => '_to_string', fallback => 1;
 
     # repository and sha1 are required initargs
@@ -74,41 +75,7 @@ class Gitalist::Git::Object with Gitalist::Git::Serializable is dirty {
     }
 
     method _build_modestr {
-        return _mode_str($self->mode);
-    }
-
-    # via gitweb.pm circa line 1305
-    use Fcntl ':mode';
-    use constant {
-        S_IFINVALID => 0030000,
-        S_IFGITLINK => 0160000,
-    };
-
-    # submodule/subrepository, a commit object reference
-    sub S_ISGITLINK($) {
-        return (($_[0] & S_IFMT) == S_IFGITLINK)
-    }
-
-    # convert file mode in octal to symbolic file mode string
-    sub _mode_str {
-        my $mode = shift;
-
-        if (S_ISGITLINK($mode)) {
-            return 'm---------';
-        } elsif (S_ISDIR($mode & S_IFMT)) {
-            return 'drwxr-xr-x';
-        } elsif ($^O ne 'MSWin32' and S_ISLNK($mode)) { # this is ENOLINKS country, we can't stop here!
-            return 'lrwxrwxrwx';
-        } elsif (S_ISREG($mode)) {
-            # git cares only about the executable bit
-            if ($mode & S_IXUSR) {
-                return '-rwxr-xr-x';
-            } else {
-                return '-rw-r--r--';
-            }
-        } else {
-            return '----------';
-        }
+        return mode_string($self->mode);
     }
 
 } # end class
index 9421c77..6e1b4fe 100644 (file)
@@ -29,6 +29,10 @@ role Gitalist::Git::Object::HasTree {
         return \@ret;
     }
 
+    method entries {
+        return $self->{_gpp_obj}->{directory_entries};
+    }
+
 }
 
 1;
index f11dc34..6d32446 100644 (file)
@@ -5,6 +5,7 @@ use Exporter qw/import/;
 
 our @EXPORT_OK = qw/
     age_string
+    mode_string
 /;
 
 sub age_string {
@@ -50,6 +51,40 @@ sub is_binary {
   return $_[0] !~ /^[[:print:]]+$ (?: \s ^[[:print:]]+$ )?/mx;
 }
 
+# via gitweb.pm circa line 1305
+use Fcntl ':mode';
+use constant {
+    S_IFINVALID => 0030000,
+    S_IFGITLINK => 0160000,
+};
+
+# submodule/subrepository, a commit object reference
+sub S_ISGITLINK($) {
+    return (($_[0] & S_IFMT) == S_IFGITLINK)
+}
+
+# convert file mode in octal to symbolic file mode string
+sub mode_string {
+    my $mode = shift;
+
+    if (S_ISGITLINK($mode)) {
+        return 'm---------';
+    } elsif (S_ISDIR($mode & S_IFMT)) {
+        return 'drwxr-xr-x';
+    } elsif ($^O ne 'MSWin32' and S_ISLNK($mode)) { # this is ENOLINKS country, we can't stop here!
+        return 'lrwxrwxrwx';
+    } elsif (S_ISREG($mode)) {
+        # git cares only about the executable bit
+        if ($mode & S_IXUSR) {
+            return '-rwxr-xr-x';
+        } else {
+            return '-rw-r--r--';
+        }
+    } else {
+        return '----------';
+    }
+}
+
 1;
 
 __END__
index d83e358..f2d63a5 100755 (executable)
@@ -13,8 +13,8 @@
        # sort files and folders
        SET tree_files          = [];
        SET tree_folders        = [];
-       FOREACH item IN tree_list;
-               IF item.type == "blob";
+       FOREACH item IN entries;
+               IF item.mode != "40000";
                        tree_files.push(item);
                ELSE;
                        tree_folders.push(item);
 %]
 
 [% BLOCK output_tree %]
-       [% FOREACH item IN tree_type.sort('file') %]
+       [% FOREACH item IN tree_type.sort('filename') %]
        <tr [% "class='invert'" IF counter % 2 %]>
-               <td class='file-mode'>[% item.modestr %]</td>
+               <td class='file-mode'>[% c.stash.mode_string(item.mode) %]</td>
          [%-
-              action_type     = item.type == 'tree' ? 'tree' : 'blob';
-              action_for_link = item.type == 'tree' ? '/ref/tree' : '/ref/blob';
-              blob_or_tree_link = c.uri_for_action(action_for_link, c.req.captures, c.req.args.to_path(item.file))
+              action_type     = item.mode == '40000' ? 'tree' : 'blob';
+              action_for_link = item.mode == '40000' ? '/ref/tree' : '/ref/blob';
+              blob_or_tree_link = c.uri_for_action(action_for_link, c.req.captures, c.req.args.to_path(item.filename))
          -%]
-               <td class="file-name"><a href="[% blob_or_tree_link %]" class="[% item.type == 'blob' ? 'file' : 'folder' %]">[% item.file %]</a></td>
+               <td class="file-name"><a href="[% blob_or_tree_link %]" class="[% item.mode == '40000' ? 'folder' : 'file' %]">[% item.filename %]</a></td>
                <td class='action-list'>
        <a href="[% blob_or_tree_link %]">[% theact %]</a>
-         [% IF item.type == 'blob' %]
-       <a href="[% c.uri_for_action('/ref/blob', c.req.captures, c.req.args.to_path(item.file)) %]" title="Blob" class="button blob">Blob</a>
-       <a href="[% c.uri_for_action('/ref/raw', c.req.captures, c.req.args.to_path(item.file)) %]" title="Raw" class="button raw">raw</a>
-       <a href="[% c.uri_for_action('/ref/blame', c.req.captures, c.req.args.to_path(item.file)) %]" title="Blame" class="button blame">blame</a>
+         [% IF item.mode != '40000' %]
+       <a href="[% c.uri_for_action('/ref/blob', c.req.captures, c.req.args.to_path(item.filename)) %]" title="Blob" class="button blob">Blob</a>
+       <a href="[% c.uri_for_action('/ref/raw', c.req.captures, c.req.args.to_path(item.filename)) %]" title="Raw" class="button raw">raw</a>
+       <a href="[% c.uri_for_action('/ref/blame', c.req.captures, c.req.args.to_path(item.filename)) %]" title="Blame" class="button blame">blame</a>
          [% END %]
-           <a href="[% c.uri_for_action('/ref/history', c.req.captures, c.req.args.to_path(item.file)) %]" title="History (Short log)" class="button shortlog">Short log</a>
+           <a href="[% c.uri_for_action('/ref/history', c.req.captures, c.req.args.to_path(item.filename)) %]" title="History (Short log)" class="button shortlog">Short log</a>
                </td>
-          <td class="message"><span class='js-data'>[% c.req.args.to_path(item.file) %]</span>Loading commit info ...</td>
+          <td class="message"><span class='js-data'>[% c.req.args.to_path(item.filename) %]</span>Loading commit info ...</td>
        </tr>
                [% counter = counter + 1 %]
        [% END %]