Hi,
On Sun, Aug 11, 2013 at 5:29 AM, Eli Carter <[email protected]> wrote:
> part3: uncache previous_rev()
> Makes previous_rev() unconditionally go to the repository without
> hitting the node_change table. (Was "part3a".)
>
> part4: warn on slow next_rev()
> This logs a warning if next_rev() takes more than 10 seconds to
> complete, suggesting enabling [trac] debug_sql. (Was "part3b".)
> The purpose of this is to give the admin that first breadcrumb to
> lead them to the underlying cause of their slowness.
I tried performance tuning of `_next_prev_rev`, tuning-_next_prev_rev.diff.
a. If the node is a directory, don't check changes of node children.
b. In condition for deletion of path ancestors, first check change_type.
The check for `path IN (...)` is expensive, if the path is deep.
c. In condition for deletion of path ancestors, don't check path itself.
The checking of path itself is included in the following.
# changes on path itself
sql += " AND (path=%s"
args.append(path)
Timing of `_next_prev_rev` with source:trunk/tracopt/mimeview/tests/php.py@8802
of a clone of http://svn.edgewall.com/repos/trac.
|| Database || before || after ||
|| || < || > || < || > ||
direction parameter
|| SQLite 3.3.6 || 0.209179s|| 0.045196s|| 0.103258s|| 0.024124s||
|| MySQL 5.0.95 || 0.230615s|| 0.236727s|| 0.203341s|| 0.214520s||
innodb_buffer_pool_size = 32M
|| PostgreSQL 8.1.23 || 0.085839s|| 0.023007s|| 0.072130s|| 0.018516s||
Thoughts?
BTW, I think that the condition for deletion of path ancestors should also
check the move of path ancestors.
change_type IN ('D', 'M')
instead of
change_type='D'
--
Jun Omae <[email protected]> (大前 潤)
--
You received this message because you are subscribed to the Google Groups "Trac
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To post to this group, send email to [email protected].
Visit this group at http://groups.google.com/group/trac-dev.
For more options, visit https://groups.google.com/groups/opt_out.
diff --git a/trac/versioncontrol/cache.py b/trac/versioncontrol/cache.py
index 9967d99..a6e5195 100644
--- a/trac/versioncontrol/cache.py
+++ b/trac/versioncontrol/cache.py
@@ -20,7 +20,8 @@ from trac.cache import cached
from trac.core import TracError
from trac.util.datefmt import from_utimestamp, to_utimestamp
from trac.util.translation import _
-from trac.versioncontrol import Changeset, Node, Repository, NoSuchChangeset
+from trac.versioncontrol.api import Changeset, Node, Repository, \
+ NoSuchChangeset, NoSuchNode
_kindmap = {'D': Node.DIRECTORY, 'F': Node.FILE}
@@ -361,28 +362,41 @@ class CachedRepository(Repository):
return self.repos.next_rev(self.normalize_rev(rev), path)
def _next_prev_rev(self, direction, rev, path=''):
+ path = self.normalize_path(path)
srev = self.db_rev(rev)
- db = self.env.get_db_cnx()
+ db = self.env.get_read_db()
# the changeset revs are sequence of ints:
sql = "SELECT rev FROM node_change WHERE repos=%s AND " + \
"rev" + direction + "%s"
args = [self.id, srev]
+ path = path.lstrip('/')
if path:
- path = path.lstrip('/')
- # changes on path itself or its children
- sql += " AND (path=%s OR path " + db.like()
- args.extend((path, db.like_escape(path + '/') + '%'))
- # deletion of path ancestors
- components = path.lstrip('/').split('/')
- parents = ','.join(('%s',) * len(components))
- sql += " OR (path IN (" + parents + ") AND change_type='D'))"
- for i in range(1, len(components) + 1):
- args.append('/'.join(components[:i]))
+ # changes on path itself
+ sql += " AND (path=%s"
+ args.append(path)
+ # changes on path children if a directory
+ try:
+ node = self.get_node(path, rev)
+ except NoSuchNode:
+ # if the node is not existent, don't check the children
+ # because changes on path itself are expected
+ node = None
+ if node and node.isdir:
+ sql += " OR path " + db.like()
+ args.append(db.like_escape(path + '/') + '%')
+ # deletion of path ancestors except itself
+ components = path.split('/')[:-1]
+ if components:
+ parents = ','.join(('%s',) * len(components))
+ sql += " OR (change_type='D' AND path IN (" + parents + "))"
+ for i in range(1, len(components) + 1):
+ args.append('/'.join(components[:i]))
+ sql += ")"
sql += " ORDER BY rev" + (direction == '<' and " DESC" or "") \
+ " LIMIT 1"
-
+
cursor = db.cursor()
cursor.execute(sql, args)
for rev, in cursor:
