Rev 5152: (andrew) Read-lock the trees and branches only once in cmd_diff. in file:///home/pqm/archives/thelove/bzr/%2Btrunk/
Canonical.com Patch Queue Manager
pqm at pqm.ubuntu.com
Wed Apr 14 05:35:49 BST 2010
At file:///home/pqm/archives/thelove/bzr/%2Btrunk/
------------------------------------------------------------
revno: 5152 [merge]
revision-id: pqm at pqm.ubuntu.com-20100414043547-j4t4napw7duy07if
parent: pqm at pqm.ubuntu.com-20100413091953-ow6ds0g52xn734v5
parent: andrew.bennetts at canonical.com-20100414031837-k7q2eyughmqxln2f
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Wed 2010-04-14 05:35:47 +0100
message:
(andrew) Read-lock the trees and branches only once in cmd_diff.
modified:
NEWS NEWS-20050323055033-4e00b5db738777ff
bzrlib/builtins.py builtins.py-20050830033751-fc01482b9ca23183
bzrlib/diff.py diff.py-20050309040759-26944fbbf2ebbf36
bzrlib/tests/test_diff.py testdiff.py-20050727164403-d1a3496ebb12e339
=== modified file 'NEWS'
--- a/NEWS 2010-04-13 09:19:53 +0000
+++ b/NEWS 2010-04-14 04:35:47 +0000
@@ -51,6 +51,10 @@
generated by a template and not edited by the user.
(Robert Collins, #530265)
+* ``bzr diff`` read-locks the trees and branches only once, saving about
+ 10-20ms on ``bzr diff`` in a bzr.dev tree.
+ (Andrew Bennetts)
+
* ``bzr missing`` read-locks the branches only once.
(Andrew Bennetts)
@@ -73,6 +77,10 @@
API Changes
***********
+* ``bzrlib.diff.get_trees_and_branches_to_diff`` is deprecated. Use
+ ``get_trees_and_branches_to_diff_locked`` instead.
+ (Andrew Bennetts)
+
Internals
*********
=== modified file 'bzrlib/builtins.py'
--- a/bzrlib/builtins.py 2010-04-13 09:19:53 +0000
+++ b/bzrlib/builtins.py 2010-04-14 04:35:47 +0000
@@ -1955,7 +1955,7 @@
@display_command
def run(self, revision=None, file_list=None, diff_options=None,
prefix=None, old=None, new=None, using=None, format=None):
- from bzrlib.diff import (get_trees_and_branches_to_diff,
+ from bzrlib.diff import (get_trees_and_branches_to_diff_locked,
show_diff_trees)
if (prefix is None) or (prefix == '0'):
@@ -1982,8 +1982,8 @@
(old_tree, new_tree,
old_branch, new_branch,
- specific_files, extra_trees) = get_trees_and_branches_to_diff(
- file_list, revision, old, new, apply_view=True)
+ specific_files, extra_trees) = get_trees_and_branches_to_diff_locked(
+ file_list, revision, old, new, self.add_cleanup, apply_view=True)
return show_diff_trees(old_tree, new_tree, sys.stdout,
specific_files=specific_files,
external_diff_options=diff_options,
=== modified file 'bzrlib/diff.py'
--- a/bzrlib/diff.py 2010-04-05 21:56:39 +0000
+++ b/bzrlib/diff.py 2010-04-13 13:24:12 +0000
@@ -32,6 +32,7 @@
branch as _mod_branch,
bzrdir,
cmdline,
+ cleanup,
errors,
osutils,
patiencediff,
@@ -48,6 +49,7 @@
)
from bzrlib.symbol_versioning import (
deprecated_function,
+ deprecated_in,
)
from bzrlib.trace import mutter, note, warning
@@ -289,6 +291,7 @@
new_abspath, e)
+ at deprecated_function(deprecated_in((2, 2, 0)))
def get_trees_and_branches_to_diff(path_list, revision_specs, old_url, new_url,
apply_view=True):
"""Get the trees and specific files to diff given a list of paths.
@@ -313,7 +316,44 @@
:returns:
a tuple of (old_tree, new_tree, old_branch, new_branch,
specific_files, extra_trees) where extra_trees is a sequence of
- additional trees to search in for file-ids.
+ additional trees to search in for file-ids. The trees and branches
+ are not locked.
+ """
+ op = cleanup.OperationWithCleanups(get_trees_and_branches_to_diff_locked)
+ return op.run_simple(path_list, revision_specs, old_url, new_url,
+ op.add_cleanup, apply_view=apply_view)
+
+
+def get_trees_and_branches_to_diff_locked(
+ path_list, revision_specs, old_url, new_url, add_cleanup, apply_view=True):
+ """Get the trees and specific files to diff given a list of paths.
+
+ This method works out the trees to be diff'ed and the files of
+ interest within those trees.
+
+ :param path_list:
+ the list of arguments passed to the diff command
+ :param revision_specs:
+ Zero, one or two RevisionSpecs from the diff command line,
+ saying what revisions to compare.
+ :param old_url:
+ The url of the old branch or tree. If None, the tree to use is
+ taken from the first path, if any, or the current working tree.
+ :param new_url:
+ The url of the new branch or tree. If None, the tree to use is
+ taken from the first path, if any, or the current working tree.
+ :param add_cleanup:
+ a callable like Command.add_cleanup. get_trees_and_branches_to_diff
+ will register cleanups that must be run to unlock the trees, etc.
+ :param apply_view:
+ if True and a view is set, apply the view or check that the paths
+ are within it
+ :returns:
+ a tuple of (old_tree, new_tree, old_branch, new_branch,
+ specific_files, extra_trees) where extra_trees is a sequence of
+ additional trees to search in for file-ids. The trees and branches
+ will be read-locked until the cleanups registered via the add_cleanup
+ param are run.
"""
# Get the old and new revision specs
old_revision_spec = None
@@ -342,12 +382,21 @@
default_location = path_list[0]
other_paths = path_list[1:]
+ def lock_tree_or_branch(wt, br):
+ if wt is not None:
+ wt.lock_read()
+ add_cleanup(wt.unlock)
+ elif br is not None:
+ br.lock_read()
+ add_cleanup(br.unlock)
+
# Get the old location
specific_files = []
if old_url is None:
old_url = default_location
working_tree, branch, relpath = \
bzrdir.BzrDir.open_containing_tree_or_branch(old_url)
+ lock_tree_or_branch(working_tree, branch)
if consider_relpath and relpath != '':
if working_tree is not None and apply_view:
views.check_path_in_view(working_tree, relpath)
@@ -361,6 +410,7 @@
if new_url != old_url:
working_tree, branch, relpath = \
bzrdir.BzrDir.open_containing_tree_or_branch(new_url)
+ lock_tree_or_branch(working_tree, branch)
if consider_relpath and relpath != '':
if working_tree is not None and apply_view:
views.check_path_in_view(working_tree, relpath)
=== modified file 'bzrlib/tests/test_diff.py'
--- a/bzrlib/tests/test_diff.py 2010-02-23 07:43:11 +0000
+++ b/bzrlib/tests/test_diff.py 2010-04-14 03:18:37 +0000
@@ -17,7 +17,6 @@
import os
import os.path
from cStringIO import StringIO
-import errno
import subprocess
import sys
from tempfile import TemporaryFile
@@ -33,6 +32,7 @@
internal_diff,
show_diff_trees,
get_trees_and_branches_to_diff,
+ get_trees_and_branches_to_diff_locked,
)
from bzrlib.errors import BinaryFile, NoDiff, ExecutableMissing
import bzrlib.osutils as osutils
@@ -44,6 +44,7 @@
TestCaseInTempDir, TestSkipped)
from bzrlib.revisiontree import RevisionTree
from bzrlib.revisionspec import RevisionSpec
+from bzrlib.symbol_versioning import deprecated_in
from bzrlib.tests.test_win32utils import BackslashDirSeparatorFeature
@@ -1394,17 +1395,23 @@
diff_obj._prepare_files('file2-id', 'oldname2', 'newname2')
-class TestGetTreesAndBranchesToDiff(TestCaseWithTransport):
+class TestGetTreesAndBranchesToDiffLocked(TestCaseWithTransport):
+
+ def call_gtabtd(self, path_list, revision_specs, old_url, new_url):
+ """Call get_trees_and_branches_to_diff_locked. Overridden by
+ TestGetTreesAndBranchesToDiff.
+ """
+ return get_trees_and_branches_to_diff_locked(
+ path_list, revision_specs, old_url, new_url, self.addCleanup)
def test_basic(self):
tree = self.make_branch_and_tree('tree')
(old_tree, new_tree,
old_branch, new_branch,
- specific_files, extra_trees) = \
- get_trees_and_branches_to_diff(['tree'], None, None, None)
+ specific_files, extra_trees) = self.call_gtabtd(
+ ['tree'], None, None, None)
self.assertIsInstance(old_tree, RevisionTree)
- #print dir (old_tree)
self.assertEqual(_mod_revision.NULL_REVISION, old_tree.get_revision_id())
self.assertEqual(tree.basedir, new_tree.basedir)
self.assertEqual(tree.branch.base, old_branch.base)
@@ -1424,8 +1431,8 @@
RevisionSpec.from_string('2')]
(old_tree, new_tree,
old_branch, new_branch,
- specific_files, extra_trees) = \
- get_trees_and_branches_to_diff(['tree'], revisions, None, None)
+ specific_files, extra_trees) = self.call_gtabtd(
+ ['tree'], revisions, None, None)
self.assertIsInstance(old_tree, RevisionTree)
self.assertEqual("old-id", old_tree.get_revision_id())
@@ -1435,3 +1442,15 @@
self.assertEqual(tree.branch.base, new_branch.base)
self.assertIs(None, specific_files)
self.assertEqual(tree.basedir, extra_trees[0].basedir)
+
+
+class TestGetTreesAndBranchesToDiff(TestGetTreesAndBranchesToDiffLocked):
+ """Apply the tests for get_trees_and_branches_to_diff_locked to the
+ deprecated get_trees_and_branches_to_diff function.
+ """
+
+ def call_gtabtd(self, path_list, revision_specs, old_url, new_url):
+ return self.applyDeprecated(
+ deprecated_in((2, 2, 0)), get_trees_and_branches_to_diff,
+ path_list, revision_specs, old_url, new_url)
+
More information about the bazaar-commits
mailing list