trac is slow when rendering the changeset page of git with many files

[walty <walty8@…>]

If a git changeset contains a lot of files, the rendering will be very slow. Currently the system would try to do two API calls for each file in the changeset, one for the old revision and one for the new revision. For example, if the changeset contains 100 files, then 200 git API calls will be triggered.

The trigger point should be the following lines of tracopt/versioncontrol/git/git_fs.py:

                    if change != Changeset.ADD:
                        old_node = self.get_node(path, old_rev, old_historian)
                    if change != Changeset.DELETE:
                        new_node = self.get_node(path, new_rev, new_historian)

[Jun Omae]

I posted t12545-gitrepos-get-changes.diff​. In the patch, git ls-tree -r REV would be called only once for each revision instead of calling git ls-tree REV -- PATH for each revision and path.

After the patch, changeset view between two revisions with git (e.g. diff:jomae.git@8a59602e:3fd4da50) would work 2 times as fast as the previous (2059 ms ⇒ 1147 ms).

However, changeset view about a revision (e.g. [3fd4da50/jomae.git]) is still slow.

[walty <walty8@…>]

Replying to Jun Omae:

I posted t12545-gitrepos-get-changes.diff​. In the patch, git ls-tree -r REV would be called only once for each revision instead of calling git ls-tree REV -- PATH for each revision and path.

I tried to apply the patch, and the improvement is significant.

For the corresponding comparison page in the Trac regarding to ​this github diff page, the total time elapsed for class ChangesetModule ⇒ def render_html() is greatly reduced in my low-profile PC:

• before: 21.565263 seconds
• after: 2.853279 seconds

[Jun Omae]

I'll add unit tests and try to improve changeset view with single revision.

[Jun Omae]

Updated jomae.git@t12545 that improves changeset view with single revision (5119 ms → 1162 ms for [3fd4da50/jomae.git]).

[Ryan J Ollos]

Moved ticket assigned to next-stable-1.0.x since maintenance of 1.0.x is coming to a close. Please move the ticket back if it's critical to fix on 1.0.x.

[Jun Omae]

I'd like to apply [3c7ad7cf2/jomae.git] to 1.0-stable. I've encountered the same issue on production environment with Trac 1.0.x.

[Christian Boos]

As we're talking performance here, do we really need a get_ls_tree function? Looks like it's only called on old_rev and new_rev. So instead of a tree_map, we could have old_tree and new_tree (plus a check if new_rev == old_rev: new_tree = old_tree).

[Jun Omae]

GitNode() needs mode, sha1, filesize and filename. git diff-tree returns a list of (src-mode, dst-mode, src-sha1, dst-sha1, status, src-path, dst-path). However, the filesize cannot be retrieved from git diff-tree. Any trees cannot be solved.

Therefore, call of git ls-tree is needed.

[Christian Boos]

Sorry, I wasn't clear, I meant that instead of having a get_ls_tree function here, we could inline it, e.g.

tracopt/versioncontrol/git/git_fs.py

@@ -573 +573 @@
         if old_path != new_path:
             raise TracError(_("Not supported in git_fs"))
 
-        ls_tree_map = {}
         old_rev = self.normalize_rev(old_rev)
         new_rev = self.normalize_rev(new_rev)
-        target_path = old_path.strip('/')
+        if old_rev == new_rev:
+            return []
 
-        def get_ls_tree(rev, path):
-            if rev not in ls_tree_map:
-                ls_tree = self.git.ls_tree(rev, target_path, recursive=True)
-                ls_tree_map[rev] = dict((info[4], info) for info in ls_tree)
-            return ls_tree_map[rev].get(path, False)
+        target_path = old_path.strip('/')
+        old_tree = dict((info[4], info) for info in
+                        self.git.ls_tree(old_rev, target_path, recursive=True))
+        new_tree = dict((info[4], info) for info in
+                        self.git.ls_tree(new_rev, target_path, recursive=True))
 
         with self.git.get_historian(old_rev, target_path) as old_historian:
             with self.git.get_historian(new_rev, target_path) as new_historian:
@@ -600 +600 @@
 
                     if change != Changeset.ADD:
                         old_node = self._get_node(path, old_rev,
-                                                  get_ls_tree(old_rev, path),
+                                                  old_tree.get(path, False),
                                                   old_historian)
                     if change != Changeset.DELETE:
                         new_node = self._get_node(path, new_rev,
-                                                  get_ls_tree(new_rev, path),
+                                                  new_tree.get(path, False),
                                                   new_historian)
 
                     yield old_node, new_node, kind, change

I didn't test it though, I got an unrelated issue with that branch:

  File "D:\Trac\repos\1.2-stable\trac\notification\mail.py", line 44, in <module>
ImportError: cannot import name get_session_attribute

[Jun Omae]

Ah, okay. That's simple. I'll push with your suggestion. Thanks.

ImportError: cannot import name get_session_attribute

Hmm. The get_session_attribute is existing in trac.web.session since r15670 at branches/1.2-stable/trac/web/session.py@15670#/get_session_attribute​. Please retry after make clean.

[Jun Omae]

Committed in [15701] and merged in [15702,15704].