[MERGE] Use a LRUCache in LocalTransport.clone

Andrew Bennetts andrew at canonical.com
Fri Jan 4 14:41:18 GMT 2008


This is a simple change to use a LRUCache in LocalTransport.clone.  It seems
that calculating an abspath can be quite expensive.  By keeping the LRUCache I
reduce the time to pull a 300 revisions from a knit repo into a pack repo from
~32 seconds to ~26 seconds!

Specifically, I'm testing with a Twisted trunk import made with bzr-svn which is
in “Bazaar Knit Repository Format 3 (bzr 0.15)” format, and timing using this
simple-minded command:

    time (bzr init --rich-root-pack foo && cd foo && bzr pull -r 300 /tmp/twisted-trunk)

(So it's also building the working tree, so the 32s vs 26s difference slightly
understates the relative improvement).

I'm a bit surprised the difference is so huge.  Looking a kcachegrind, I see
that there's an immense number of calls to clone (63500 calls to
get_weave_or_empty, which each seem to cause a call to clone).  I wouldn't be
surprised if there's a way to avoid calling clone so much, which might be
preferable to keeping a cache.  Or perhaps clone should just be faster; by
calling self.abspath it invokes local_path_to_url, and in constructing a new
LocalTransport it also invokes local_path_from_url... it seems like one of those
ought to be unnecessary in an ideal world.

In short, I suspect this patch is treating a symptom of a deeper problem, rather
than fixing the cause.  And I don't know if many operations would benefit as
much as the one I was testing with (especially as people start to use packs
rather than knits, as packs use many less files).  But maybe it's worth having
anyway.

-Andrew.

-------------- next part --------------
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: andrew.bennetts at canonical.com-20080104141831-\
#   shst3t9bevj4rqty
# target_branch: http://bazaar-vcs.org/bzr/bzr.dev
# testament_sha1: 722ea61d995ea1bb7c74b0e470f550db18ab35d0
# timestamp: 2008-01-05 01:18:58 +1100
# source_branch: http://people.ubuntu.com/~andrew/bzr/local-transport-\
#   clone-speed
# base_revision_id: pqm at pqm.ubuntu.com-20080103231814-wymg8td870lqfzkv
# 
# Begin patch
=== modified file 'bzrlib/transport/local.py'
--- bzrlib/transport/local.py	2007-11-22 00:29:58 +0000
+++ bzrlib/transport/local.py	2008-01-04 14:18:31 +0000
@@ -30,6 +30,7 @@
 
 from bzrlib import (
     atomicfile,
+    lru_cache,
     osutils,
     urlutils,
     symbol_versioning,
@@ -45,6 +46,7 @@
 _append_flags = os.O_CREAT | os.O_APPEND | os.O_WRONLY | osutils.O_BINARY
 _put_non_atomic_flags = os.O_CREAT | os.O_TRUNC | os.O_WRONLY | osutils.O_BINARY
 
+_local_transport_lru = lru_cache.LRUCache()
 
 class LocalTransport(Transport):
     """This is the transport agent for local filesystem access."""
@@ -72,15 +74,21 @@
         we can just return a new object.
         """
         if offset is None:
-            return LocalTransport(self.base)
+            return self
         else:
-            abspath = self.abspath(offset)
-            if abspath == 'file://':
-                # fix upwalk for UNC path
-                # when clone from //HOST/path updir recursively
-                # we should stop at least at //HOST part
-                abspath = self.base
-            return LocalTransport(abspath)
+            key = (self.base, offset)
+            try:
+                t = _local_transport_lru[key]
+            except KeyError:
+                abspath = self.abspath(offset)
+                if abspath == 'file://':
+                    # fix upwalk for UNC path
+                    # when clone from //HOST/path updir recursively
+                    # we should stop at least at //HOST part
+                    abspath = self.base
+                t = LocalTransport(abspath)
+                _local_transport_lru[key] = t
+            return t
 
     def _abspath(self, relative_reference):
         """Return a path for use in os calls.

# Begin bundle
IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWVL0qccAAkx/gGBUQABZ7///
f///ur////BQBW9zYAF01KqqChCUSaNJmpMZE2k0xopvVHpppkQZPUzRqek000A0HqCJRkaMjTTC
AaGg0MhkAGmhoyGQGRoEU1PNJMTQMgaNAAAAAAaAADQDjJk0aA0aYjI0MQwJo0xBiNBhAAYJJBAA
jTTSNTyJtU/I1GTRP1Go9ENHpPU9IGgYSKwIqE9xSma0zu8vtuS/G5dkjHNMi/FhH8oabfv67KSi
o1d8lUYZ06RCjs2ue0HO3PLGxEhRSX+1c8XXfDlPfvZmQDv0QB6dE6QsHqiCxDmIRwwRciKyYNxQ
ZJ8SHmvLvhpA7SHs7aIpFhhOr+hIysPgoqCUaipeOYomQeKpAmKhjeUQCD9afGPdrp57lqy2Bi57
AjMlEAh0pl5da1loW2vVca1a/ocdAgdy9CyEpWC6TB+rkriBTX8pgzxHfXJxLyoKpuoGAkZ5YGPR
oBx8PCq/M5u43spkF7erggM4LbHEMWAcviaz6h8p2QeCniymHKaUoDs9J2B12d81NwWMojDM8eBD
ptBTTxZRG67R7Zwo/TIFplA2EI3TnfWEoG6EanUQYcZzJcYlzikzbwCm9iywqumRWbH2RRfMN/jn
zYenIYZIAaXBUVBc52BTHG/jzG0CJdKFkXFADATsVNDArLhmMw8r4/kBHiz4U4QzFy0JjbJLpV+W
l8z6TKsDO8lJ8KzPPcXkbMpcRs4pV5bY12MiixYvLCNOgi6RTFtQNqflpCYdfNkmcCYqHhSwD40u
GDHHMVFxYSE/ryDqF2BdbC628yPJhiNZmlvLiNeTkMuoCj98dvhRz9fFVDKWmKsB6ZsTMiNxr1FL
C9rGz1aUvZcTl2F9JGDOUtHj8xj53yuJ1ZAxA5gw/Ts1R7cDTyk3ngqqbTj1Taq13/ajldD0Y7Gx
d6FYJno6nA3aLBkW8cU0ucnfqWQO817kYU8DLxTvD2NglwDCQDGUCVDDjFT4OSmTPGkUpATOCT9m
hpbOwqwEdjvMkuDh69jioU5zmXQxCN/4akM+aCssyDnkKDzSSeEWPPtPYe2ayZkICDh6cxeR9+1W
fYnToIoO8rAzG+2fazEUq98vabDuTXz9RwGTbGRtQPWMA6e0kPde30MoPRHSmwYXTjb8Q1/Mu8aB
gP+QY141nHLchtWOityE/quOnvz9uJWnhqpB5XOyIDiQY0FvKpRGfnsf1N0aMkCSLt0sUXicW8QD
24afByVtNfKcQ+iuquahZZAVWqo7BjKC9b/keIZ7N+CC+SKYVdleGan20FgayokamUxSHJEoCiH6
iDJMpHRhltSNS6XVnMxCcmmEgOEvko8+tXjlNMmh4pzYfQIYBzY8nQlR8fElo5L+V70MFm/kgdzA
c+Iki+adAVdHpQhOnKYpOAIS/Qg+ro242xVhCXkQMUD2ogBudxEefZ5/L9D54o2htSJ7Acl2bgv8
wXRN0h4erUtdLaaAYm5+d5JXf51QTQBf/L4UtJZpnO0MUve7SRWRoGRysF59s0qJ3IIh7OoRHqgg
akRIggyByHMLAOt4cm+04hwyHWnkv1wYSmESLmuQR6WClnIcHDgIer35HVWGnTxg82qDETC18wdr
kFcfvDdSqKhq5NKzBOBeBXx52mPpB3qcjh+egZkV4+ShJ3mMvAfeyfwEWTVhQYY16pMAw3Ae6mjL
UIeCqUIMJVUpawVUClcA4nTJNaGsN+YiKgIMCTqJo0hpRJagtKbQHFNSNj3SwKZTsMcsIUunGZgg
B7gN+eU75pV3Ew47AeX2KcnS4pSBMKdbsmdD+aml45BO4NGUQ4K3HoCLA1TShJ3vDoDZk1k2oL3N
QvFT64Mrnq1Aa2EeGqUxCXm09cyK1oZ2DJFRRjFWBGawivEhNbB0f/F3JFOFCQUvSpxw


More information about the bazaar mailing list