Re: [PATCH] Hardlinked chroot for morph
On Tue, Nov 27, 2012 at 05:05:30PM +0000, Joe Burmeister wrote:
Patch to improve morph build time by reusing decompressed files of
chunks/stage-fillers with hardlinks from the chroot. Rather than
decompressing each time into each chroot.
Original: real 5h 17m 47s
Hardlink: real 2h 52m 27s
---
morphlib/stagingarea.py | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/morphlib/stagingarea.py b/morphlib/stagingarea.py
index a87b45c..ef41fd2 100644
--- a/morphlib/stagingarea.py
+++ b/morphlib/stagingarea.py
@@ -17,6 +17,7 @@
import logging
import os
import shutil
+import subprocess
import morphlib
@@ -90,7 +91,19 @@ class StagingArea(object):
logging.debug('Installing artifact %s' %
getattr(handle, 'name', 'unknown name'))
- morphlib.bins.unpack_binary_from_file(handle, self.dirname)
+
+ decomp = self._app.settings['tempdir'] + "/" +
os.path.basename(handle.name)
It's better to use os.path.join() for paths.
I keep having to work out what decomp means, decompressed_chroot, while
more verbose would be clearer for me at least.
+ srctime = os.stat(handle.name).st_mtime
+ if not os.path.exists(decomp) or ( (srctime - os.stat(decomp).st_mtime) >
0.001 ):
+ self._mkdir(decomp)
+ morphlib.bins.unpack_binary_from_file(handle, decomp + "/")
+ os.utime(decomp,(srctime,srctime))
A comment about what the timing is for would help here. Am I correct in
thinking that if the chroot hasn't been extracted, or the artifact is
newer, then it's created?
+ if not os.path.exists(self.dirname):
+ self._mkdir(self.dirname)
+
+ subprocess.call(["sh","-c","cp -al " +
decomp+"/* " + self.dirname+"/"])
In the rest of the codebase we use self._app.runcmd, which has roughly
the same semantics as subprocess.call, but it integrates with the
logging.
Also, rather than running with sh -c, you can set the keyword argument
shell=True to eval a command line, rather than exec it.
Also I'd really prefer if this execution didn't use either shell=True,
or sh -c, because in other contexts at least, it's a security
vulnerability.
Altering the command line to something like the following may work, but
I haven't tested it.
["cp", "-al"] + glob.glob(decomp + "/*") +
[self.dirname]
It should also be possible to do this hardlinking in-process, but cp -al
may be faster and is certainly a useful shortcut to the required
functionality.