9:25 a.m.
Repo: baserock:baserock/morph
Ref: baserock/danielsilverstone/memoise-cache-keys-better
SHA1: 6d9eee86f1ea75c0f2666dc4282d8b10fee63e7b
Land: master
This patch series adds some more memoisation which speeds up cache key
computation quite significantly. Also the tests are updated to ensure that we
actually do exercise the memoisation pathways and the logging is updated to
make it clear when we're returning memoised results.
This will need a definitions change in tools.morph to update the SHA1 for
morph.
I can confirm that ./check passes (modulo a rawdisk related yarn which I cannot
run successfully in my chroot right now anyway).
D.
Daniel Silverstone (2):
CacheKeyComputer: Memoise cache keys more effectively.
CacheKeyComputer Tests: Update to explicitly test for memoisation
morphlib/cachekeycomputer.py | 33 +++++++++++++++++++++++----------
morphlib/cachekeycomputer_tests.py | 13 +++++++++++++
2 files changed, 36 insertions(+), 10 deletions(-)
--
1.7.10.4
9:25 a.m.
New subject: [PATCH 1/2] CacheKeyComputer: Memoise cache keys more effectively.
We were previously memoising the computation of the dictionaries
but this patch adds support for memoising the computation of the
cache key itself (the SHA string). This massively improves cache
key computation performance.
Signed-Off-By: Daniel Silverstone <daniel.silverstone(a)codethink.co.uk>
Signed-off-by: Daniel Silverstone <daniel.silverstone(a)codethink.co.uk>
---
morphlib/cachekeycomputer.py | 33 +++++++++++++++++++++++----------
1 file changed, 23 insertions(+), 10 deletions(-)
diff --git a/morphlib/cachekeycomputer.py b/morphlib/cachekeycomputer.py
index 3efe1cb..ca37443 100644
--- a/morphlib/cachekeycomputer.py
+++ b/morphlib/cachekeycomputer.py
@@ -25,6 +25,7 @@ class CacheKeyComputer(object):
def __init__(self, build_env):
self._build_env = build_env
self._calculated = {}
+ self._hashed = {}
def _filterenv(self, env):
keys = ["LOGNAME", "MORPH_ARCH", "TARGET",
"TARGET_STAGE1",
@@ -32,11 +33,18 @@ class CacheKeyComputer(object):
return dict([(k, env[k]) for k in keys])
def compute_key(self, artifact):
- logging.debug('computing cache key for artifact %s from source '
- 'repo %s, sha1 %s, filename %s' %
- (artifact.name, artifact.source.repo_name,
- artifact.source.sha1, artifact.source.filename))
- return self._hash_id(self.get_cache_id(artifact))
+ try:
+ ret = self._hashed[artifact]
+ logging.debug('returning cached key for artifact %s from source ',
+ (artifact.name, artifact.source.repo_name,
+ artifact.source.sha1, artifact.source.filename))
+ return ret
+ except KeyError:
+ logging.debug('computing cache key for artifact %s from source ',
+ (artifact.name, artifact.source.repo_name,
+ artifact.source.sha1, artifact.source.filename))
+ self._hashed[artifact] = self._hash_id(self.get_cache_id(artifact))
+ return self._hashed[artifact]
def _hash_id(self, cache_id):
sha = hashlib.sha256()
@@ -66,13 +74,18 @@ class CacheKeyComputer(object):
self._hash_thing(sha, item)
def get_cache_id(self, artifact):
- logging.debug('computing cache id for artifact %s from source '
- 'repo %s, sha1 %s, filename %s' %
- (artifact.name, artifact.source.repo_name,
- artifact.source.sha1, artifact.source.filename))
try:
- return self._calculated[artifact]
+ ret = self._calculated[artifact]
+ logging.debug('returning cached id for artifact %s from source '
+ 'repo %s, sha1 %s, filename %s' %
+ (artifact.name, artifact.source.repo_name,
+ artifact.source.sha1, artifact.source.filename))
+ return ret
except KeyError:
+ logging.debug('computing cache id for artifact %s from source '
+ 'repo %s, sha1 %s, filename %s' %
+ (artifact.name, artifact.source.repo_name,
+ artifact.source.sha1, artifact.source.filename))
cacheid = self._calculate(artifact)
self._calculated[artifact] = cacheid
return cacheid
--
1.7.10.4
9:25 a.m.
New subject: [PATCH 2/2] CacheKeyComputer Tests: Update to explicitly test for memoisation
Explicitly test that we get the same result twice when computing
ids and keys for the same artifact.
This has the effect of verifying that the memoisation code is
correctly operating.
Signed-Off-By: Daniel Silverstone <daniel.silverstone(a)codethink.co.uk>
Signed-off-by: Daniel Silverstone <daniel.silverstone(a)codethink.co.uk>
---
morphlib/cachekeycomputer_tests.py | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/morphlib/cachekeycomputer_tests.py b/morphlib/cachekeycomputer_tests.py
index 4e73e90..dd10307 100644
--- a/morphlib/cachekeycomputer_tests.py
+++ b/morphlib/cachekeycomputer_tests.py
@@ -148,6 +148,19 @@ class CacheKeyComputerTests(unittest.TestCase):
validchars = '0123456789abcdef'
return len(s) == 64 and all([c in validchars for c in s])
+ def test_compute_twice_same_key(self):
+ artifact = self._find_artifact('system-rootfs')
+ self.assertEqual(self.ckc.compute_key(artifact),
+ self.ckc.compute_key(artifact))
+
+ def test_compute_twice_same_id(self):
+ artifact = self._find_artifact('system-rootfs')
+ id1 = self.ckc.get_cache_id(artifact)
+ id2 = self.ckc.get_cache_id(artifact)
+ hash1 = self.ckc._hash_id(id1)
+ hash2 = self.ckc._hash_id(id2)
+ self.assertEqual(hash1, hash2)
+
def test_compute_key_returns_sha256(self):
artifact = self._find_artifact('system-rootfs')
self.assertTrue(self._valid_sha256(
--
1.7.10.4
9:59 a.m.
On Fri, Mar 28, 2014 at 02:25:51PM +0000, Daniel Silverstone wrote:
Repo: baserock:baserock/morph
Ref: baserock/danielsilverstone/memoise-cache-keys-better
SHA1: 6d9eee86f1ea75c0f2666dc4282d8b10fee63e7b
Land: master
This patch series adds some more memoisation which speeds up cache key
computation quite significantly. Also the tests are updated to ensure that we
actually do exercise the memoisation pathways and the logging is updated to
make it clear when we're returning memoised results.
This will need a definitions change in tools.morph to update the SHA1 for
morph.
I can confirm that ./check passes (modulo a rawdisk related yarn which I cannot
run successfully in my chroot right now anyway).
+2
My devel VM is busy so I haven't been able to test this, but I trust
Daniel to DTRT when merging.
--
http://www.codethink.co.uk/ http://wiki.baserock.org/ http://www.baserock.com/
10:03 a.m.
On Fri, Mar 28, 2014 at 14:59:09 +0000, Lars Wirzenius wrote:
+2
My devel VM is busy so I haven't been able to test this, but I trust
Daniel to DTRT when merging.
Thanks Lars, I've merged this to morph's master and also applied the following:
diff --git a/tools.morph b/tools.morph
index dd5d79d..77f17a2 100644
--- a/tools.morph
+++ b/tools.morph
@@ -92,7 +92,7 @@ chunks:
- six
- name: morph
repo: baserock:baserock/morph
- ref: f56259c2ce62c301f0e0f9dd7c85001b45fc8f5c
+ ref: 8db794a8e9565fbc9dda993fc7347436a53dddd4
unpetrify-ref: master
build-depends:
- cliapp
to the master of the definitions repository.
Everyone should enjoy faster cache key computation in their new systems soon :-)
D.
--
Daniel Silverstone http://www.codethink.co.uk/
Lead Architect GPG 4096/R Key Id: 3CCE BABE 206C 3B69
11:04 a.m.
On 28/03/14 15:03, Daniel Silverstone wrote:
On Fri, Mar 28, 2014 at 14:59:09 +0000, Lars Wirzenius wrote:
> +2
>
> My devel VM is busy so I haven't been able to test this, but I trust
> Daniel to DTRT when merging.
Thanks Lars, I've merged this to morph's master and also applied the following:
diff --git a/tools.morph b/tools.morph
index dd5d79d..77f17a2 100644
--- a/tools.morph
+++ b/tools.morph
@@ -92,7 +92,7 @@ chunks:
- six
- name: morph
repo: baserock:baserock/morph
- ref: f56259c2ce62c301f0e0f9dd7c85001b45fc8f5c
+ ref: 8db794a8e9565fbc9dda993fc7347436a53dddd4
unpetrify-ref: master
build-depends:
- cliapp
to the master of the definitions repository.
Everyone should enjoy faster cache key computation in their new systems soon :-)
Ran a build; the "Collecting morphologies involved in building
base-system-x86_32-generic from master" is now much faster.
--
Sam Thursfield, Codethink Ltd.
Office telephone: +44 161 236 5575
2:07 p.m.
On 28/03/14 14:25, Daniel Silverstone wrote:
Repo: baserock:baserock/morph
Ref: baserock/danielsilverstone/memoise-cache-keys-better
SHA1: 6d9eee86f1ea75c0f2666dc4282d8b10fee63e7b
Land: master
This patch series adds some more memoisation which speeds up cache key
computation quite significantly. Also the tests are updated to ensure that we
actually do exercise the memoisation pathways and the logging is updated to
make it clear when we're returning memoised results.
This will need a definitions change in tools.morph to update the SHA1 for
morph.
I can confirm that ./check passes (modulo a rawdisk related yarn which I cannot
run successfully in my chroot right now anyway).
Sorry to report that the 'Computing build graph' step in distbuild
still takes an unreasonable amount of time. :(
3283
days inactive
3283
days old
6 comments
4 participants
participants (4)
-
Daniel Silverstone -
Lars Wirzenius -
Richard Ipsum -
Sam Thursfield