Merge branch 'juerg/fast-import' into 'master'
Improve import performance
See merge request BuildStream/buildstream!1190
diff --git a/buildstream/_artifactcache.py b/buildstream/_artifactcache.py
index 9986d68..3303650 100644
--- a/buildstream/_artifactcache.py
+++ b/buildstream/_artifactcache.py
@@ -692,9 +692,8 @@
# logsdir (CasBasedDirectory): A CasBasedDirectory containing the artifact's logs
#
def get_artifact_logs(self, ref):
- descend = ["logs"]
cache_id = self.cas.resolve_ref(ref, update_mtime=True)
- vdir = CasBasedDirectory(self.cas, digest=cache_id).descend(descend)
+ vdir = CasBasedDirectory(self.cas, digest=cache_id).descend('logs')
return vdir
################################################
diff --git a/buildstream/element.py b/buildstream/element.py
index 9d13337..47ca04c 100644
--- a/buildstream/element.py
+++ b/buildstream/element.py
@@ -178,6 +178,12 @@
*Since: 1.4*
"""
+ BST_RUN_COMMANDS = True
+ """Whether the element may run commands using Sandbox.run.
+
+ *Since: 1.4*
+ """
+
def __init__(self, context, project, meta, plugin_conf):
self.__cache_key_dict = None # Dict for cache key calculation
@@ -663,14 +669,14 @@
with self.timed_activity("Staging {}/{}".format(self.name, self._get_brief_display_key())):
artifact_vdir, _ = self.__get_artifact_directory()
- files_vdir = artifact_vdir.descend(['files'])
+ files_vdir = artifact_vdir.descend('files')
# Hard link it into the staging area
#
vbasedir = sandbox.get_virtual_directory()
vstagedir = vbasedir \
if path is None \
- else vbasedir.descend(path.lstrip(os.sep).split(os.sep))
+ else vbasedir.descend(*path.lstrip(os.sep).split(os.sep))
split_filter = self.__split_filter_func(include, exclude, orphans)
@@ -1439,7 +1445,7 @@
# Stage all sources that need to be copied
sandbox_vroot = sandbox.get_virtual_directory()
- host_vdirectory = sandbox_vroot.descend(directory.lstrip(os.sep).split(os.sep), create=True)
+ host_vdirectory = sandbox_vroot.descend(*directory.lstrip(os.sep).split(os.sep), create=True)
self._stage_sources_at(host_vdirectory, mount_workspaces=mount_workspaces, usebuildtree=sandbox._usebuildtree)
# _stage_sources_at():
@@ -1478,7 +1484,7 @@
# Check if we have a cached buildtree to use
elif usebuildtree:
artifact_vdir, _ = self.__get_artifact_directory()
- import_dir = artifact_vdir.descend(['buildtree'])
+ import_dir = artifact_vdir.descend('buildtree')
if import_dir.is_empty():
detail = "Element type either does not expect a buildtree or it was explictily cached without one."
self.warn("WARNING: {} Artifact contains an empty buildtree".format(self.name), detail=detail)
@@ -1624,6 +1630,13 @@
with _signals.terminator(cleanup_rootdir), \
self.__sandbox(rootdir, output_file, output_file, self.__sandbox_config) as sandbox: # noqa
+ if not self.BST_RUN_COMMANDS:
+ # Element doesn't need to run any commands in the sandbox.
+ #
+ # Disable Sandbox.run() to allow CasBasedDirectory for all
+ # sandboxes.
+ sandbox._disable_run()
+
# By default, the dynamic public data is the same as the static public data.
# The plugin's assemble() method may modify this, though.
self.__dynamic_public = _yaml.node_copy(self.__public)
@@ -1663,7 +1676,7 @@
if workspace and self.__staged_sources_directory:
sandbox_vroot = sandbox.get_virtual_directory()
path_components = self.__staged_sources_directory.lstrip(os.sep).split(os.sep)
- sandbox_vpath = sandbox_vroot.descend(path_components)
+ sandbox_vpath = sandbox_vroot.descend(*path_components)
try:
sandbox_vpath.import_files(workspace.get_absolute_path())
except UtilError as e2:
@@ -1684,7 +1697,7 @@
if collect is not None:
try:
sandbox_vroot = sandbox.get_virtual_directory()
- collectvdir = sandbox_vroot.descend(collect.lstrip(os.sep).split(os.sep))
+ collectvdir = sandbox_vroot.descend(*collect.lstrip(os.sep).split(os.sep))
except VirtualDirectoryError:
# No collect directory existed
collectvdir = None
@@ -1721,7 +1734,7 @@
sandbox_vroot = sandbox.get_virtual_directory()
try:
sandbox_build_dir = sandbox_vroot.descend(
- self.get_variable('build-root').lstrip(os.sep).split(os.sep))
+ *self.get_variable('build-root').lstrip(os.sep).split(os.sep))
buildtreevdir.import_files(sandbox_build_dir)
except VirtualDirectoryError:
# Directory could not be found. Pre-virtual
@@ -2645,7 +2658,7 @@
filter_func = self.__split_filter_func(include=include, exclude=exclude, orphans=orphans)
artifact_vdir, _ = self.__get_artifact_directory()
- files_vdir = artifact_vdir.descend(['files'])
+ files_vdir = artifact_vdir.descend('files')
element_files = files_vdir.list_relative_paths()
diff --git a/buildstream/plugins/elements/compose.py b/buildstream/plugins/elements/compose.py
index 12520ce..f45ffd7 100644
--- a/buildstream/plugins/elements/compose.py
+++ b/buildstream/plugins/elements/compose.py
@@ -156,7 +156,7 @@
# instead of into a subdir. The element assemble() method should
# support this in some way.
#
- installdir = vbasedir.descend(['buildstream', 'install'], create=True)
+ installdir = vbasedir.descend('buildstream', 'install', create=True)
# We already saved the manifest for created files in the integration phase,
# now collect the rest of the manifest.
diff --git a/buildstream/plugins/elements/filter.py b/buildstream/plugins/elements/filter.py
index 5a2db2c..232f4cc 100644
--- a/buildstream/plugins/elements/filter.py
+++ b/buildstream/plugins/elements/filter.py
@@ -160,6 +160,12 @@
# added, to reduce the potential for confusion
BST_FORBID_SOURCES = True
+ # This plugin has been modified to avoid the use of Sandbox.get_directory
+ BST_VIRTUAL_DIRECTORY = True
+
+ # Filter elements do not run any commands
+ BST_RUN_COMMANDS = False
+
def configure(self, node):
self.node_validate(node, [
'include', 'exclude', 'include-orphans'
diff --git a/buildstream/plugins/elements/import.py b/buildstream/plugins/elements/import.py
index 3ae979d..7388421 100644
--- a/buildstream/plugins/elements/import.py
+++ b/buildstream/plugins/elements/import.py
@@ -41,6 +41,9 @@
# This plugin has been modified to avoid the use of Sandbox.get_directory
BST_VIRTUAL_DIRECTORY = True
+ # Import elements do not run any commands
+ BST_RUN_COMMANDS = False
+
def configure(self, node):
self.node_validate(node, [
'source', 'target'
@@ -75,14 +78,14 @@
self._stage_sources_in_sandbox(sandbox, 'input', mount_workspaces=False)
rootdir = sandbox.get_virtual_directory()
- inputdir = rootdir.descend(['input'])
- outputdir = rootdir.descend(['output'], create=True)
+ inputdir = rootdir.descend('input')
+ outputdir = rootdir.descend('output', create=True)
# The directory to grab
- inputdir = inputdir.descend(self.source.strip(os.sep).split(os.sep))
+ inputdir = inputdir.descend(*self.source.strip(os.sep).split(os.sep))
# The output target directory
- outputdir = outputdir.descend(self.target.strip(os.sep).split(os.sep), create=True)
+ outputdir = outputdir.descend(*self.target.strip(os.sep).split(os.sep), create=True)
if inputdir.is_empty():
raise ElementError("{}: No files were found inside directory '{}'"
diff --git a/buildstream/plugins/elements/stack.py b/buildstream/plugins/elements/stack.py
index 138afed..b26281f 100644
--- a/buildstream/plugins/elements/stack.py
+++ b/buildstream/plugins/elements/stack.py
@@ -63,7 +63,7 @@
# the actual artifact data in a subdirectory, then we
# will be able to store some additional state in the
# artifact cache, and we can also remove this hack.
- vrootdir.descend(['output', 'bst'], create=True)
+ vrootdir.descend('output', 'bst', create=True)
# And we're done
return '/output'
diff --git a/buildstream/sandbox/_sandboxremote.py b/buildstream/sandbox/_sandboxremote.py
index 9ca4738..348ebca 100644
--- a/buildstream/sandbox/_sandboxremote.py
+++ b/buildstream/sandbox/_sandboxremote.py
@@ -317,7 +317,7 @@
for mark in self._get_marked_directories():
directory = mark['directory']
# Create each marked directory
- upload_vdir.descend(directory.split(os.path.sep), create=True)
+ upload_vdir.descend(*directory.split(os.path.sep), create=True)
# Generate action_digest first
input_root_digest = upload_vdir._get_digest()
diff --git a/buildstream/sandbox/sandbox.py b/buildstream/sandbox/sandbox.py
index ca46525..f11ddea 100644
--- a/buildstream/sandbox/sandbox.py
+++ b/buildstream/sandbox/sandbox.py
@@ -117,6 +117,7 @@
self.__env = None
self.__mount_sources = {}
self.__allow_real_directory = kwargs['allow_real_directory']
+ self.__allow_run = True
# Plugin ID for logging
plugin = kwargs.get('plugin', None)
@@ -190,8 +191,9 @@
"""
if self._vdir is None or self._never_cache_vdirs:
- if 'BST_CAS_DIRECTORIES' in os.environ:
- self._vdir = CasBasedDirectory(self.__context.artifactcache.cas)
+ if self._use_cas_based_directory():
+ cascache = self.__context.get_cascache()
+ self._vdir = CasBasedDirectory(cascache)
else:
self._vdir = FileBasedDirectory(self._root)
return self._vdir
@@ -278,6 +280,9 @@
not exist yet, even if a workspace is being used.
"""
+ if not self.__allow_run:
+ raise SandboxError("Sandbox.run() has been disabled")
+
# Fallback to the sandbox default settings for
# the cwd and env.
#
@@ -386,6 +391,22 @@
def _create_batch(self, main_group, flags, *, collect=None):
return _SandboxBatch(self, main_group, flags, collect=collect)
+ # _use_cas_based_directory()
+ #
+ # Whether to use CasBasedDirectory as sandbox root. If this returns `False`,
+ # FileBasedDirectory will be used.
+ #
+ # Returns:
+ # (bool): Whether to use CasBasedDirectory
+ #
+ def _use_cas_based_directory(self):
+ # Use CasBasedDirectory as sandbox root if neither Sandbox.get_directory()
+ # nor Sandbox.run() are required. This allows faster staging.
+ if not self.__allow_real_directory and not self.__allow_run:
+ return True
+
+ return 'BST_CAS_DIRECTORIES' in os.environ
+
################################################
# Private methods #
################################################
@@ -562,6 +583,15 @@
else:
callback()
+ # _disable_run()
+ #
+ # Raise exception if `Sandbox.run()` is called. This enables use of
+ # CasBasedDirectory for faster staging when command execution is not
+ # required.
+ #
+ def _disable_run(self):
+ self.__allow_run = False
+
# _SandboxBatch()
#
diff --git a/buildstream/scriptelement.py b/buildstream/scriptelement.py
index 697cd28..3327f81 100644
--- a/buildstream/scriptelement.py
+++ b/buildstream/scriptelement.py
@@ -249,7 +249,7 @@
.format(element.name, item['destination']),
silent_nested=True):
virtual_dstdir = sandbox.get_virtual_directory()
- virtual_dstdir.descend(item['destination'].lstrip(os.sep).split(os.sep), create=True)
+ virtual_dstdir.descend(*item['destination'].lstrip(os.sep).split(os.sep), create=True)
element.stage_dependency_artifacts(sandbox, Scope.RUN, path=item['destination'])
with sandbox.batch(SandboxFlags.NONE):
@@ -269,7 +269,7 @@
dep.integrate(sandbox)
install_root_path_components = self.__install_root.lstrip(os.sep).split(os.sep)
- sandbox.get_virtual_directory().descend(install_root_path_components, create=True)
+ sandbox.get_virtual_directory().descend(*install_root_path_components, create=True)
def assemble(self, sandbox):
diff --git a/buildstream/storage/_casbaseddirectory.py b/buildstream/storage/_casbaseddirectory.py
index 43497ea..91e1188 100644
--- a/buildstream/storage/_casbaseddirectory.py
+++ b/buildstream/storage/_casbaseddirectory.py
@@ -55,6 +55,12 @@
return self.buildstream_object
+ def get_digest(self):
+ if self.digest:
+ return self.digest
+ else:
+ return self.buildstream_object._get_digest()
+
class ResolutionException(VirtualDirectoryError):
""" Superclass of all exceptions that can be raised by
@@ -155,8 +161,9 @@
def _add_file(self, basename, filename, modified=False):
entry = IndexEntry(filename, _FileType.REGULAR_FILE,
modified=modified or filename in self.index)
- entry.digest = self.cas_cache.add_object(path=os.path.join(basename, filename))
- entry.is_executable = os.access(os.path.join(basename, filename), os.X_OK)
+ path = os.path.join(basename, filename)
+ entry.digest = self.cas_cache.add_object(path=path)
+ entry.is_executable = os.access(path, os.X_OK)
self.index[filename] = entry
self.__invalidate_digest()
@@ -175,13 +182,12 @@
self.__invalidate_digest()
- def descend(self, subdirectory_spec, create=False):
+ def descend(self, *paths, create=False):
"""Descend one or more levels of directory hierarchy and return a new
Directory object for that directory.
Arguments:
- * subdirectory_spec (list of strings): A list of strings which are all directory
- names.
+ * *paths (str): A list of strings which are all directory names.
* create (boolean): If this is true, the directories will be created if
they don't already exist.
@@ -193,47 +199,38 @@
"""
- # It's very common to send a directory name instead of a list and this causes
- # bizarre errors, so check for it here
- if not isinstance(subdirectory_spec, list):
- subdirectory_spec = [subdirectory_spec]
+ current_dir = self
- # Because of the way split works, it's common to get a list which begins with
- # an empty string. Detect these and remove them.
- while subdirectory_spec and subdirectory_spec[0] == "":
- subdirectory_spec.pop(0)
+ for path in paths:
+ # Skip empty path segments
+ if not path:
+ continue
- # Descending into [] returns the same directory.
- if not subdirectory_spec:
- return self
-
- if subdirectory_spec[0] in self.index:
- entry = self.index[subdirectory_spec[0]]
- if entry.type == _FileType.DIRECTORY:
- subdir = entry.get_directory(self)
- return subdir.descend(subdirectory_spec[1:], create)
+ entry = current_dir.index.get(path)
+ if entry:
+ if entry.type == _FileType.DIRECTORY:
+ current_dir = entry.get_directory(current_dir)
+ else:
+ error = "Cannot descend into {}, which is a '{}' in the directory {}"
+ raise VirtualDirectoryError(error.format(path,
+ current_dir.index[path].type,
+ current_dir))
else:
- error = "Cannot descend into {}, which is a '{}' in the directory {}"
- raise VirtualDirectoryError(error.format(subdirectory_spec[0],
- self.index[subdirectory_spec[0]].type,
- self))
- else:
- if create:
- newdir = self._add_directory(subdirectory_spec[0])
- return newdir.descend(subdirectory_spec[1:], create)
- else:
- error = "'{}' not found in {}"
- raise VirtualDirectoryError(error.format(subdirectory_spec[0], str(self)))
- return None
+ if create:
+ current_dir = current_dir._add_directory(path)
+ else:
+ error = "'{}' not found in {}"
+ raise VirtualDirectoryError(error.format(path, str(current_dir)))
- def _check_replacement(self, name, path_prefix, fileListResult):
+ return current_dir
+
+ def _check_replacement(self, name, relative_pathname, fileListResult):
""" Checks whether 'name' exists, and if so, whether we can overwrite it.
If we can, add the name to 'overwritten_files' and delete the existing entry.
Returns 'True' if the import should go ahead.
fileListResult.overwritten and fileListResult.ignore are updated depending
on the result. """
existing_entry = self.index.get(name)
- relative_pathname = os.path.join(path_prefix, name)
if existing_entry is None:
return True
elif existing_entry.type == _FileType.DIRECTORY:
@@ -285,11 +282,11 @@
continue
if direntry.is_file(follow_symlinks=False):
- if self._check_replacement(direntry.name, path_prefix, result):
+ if self._check_replacement(direntry.name, relative_pathname, result):
self._add_file(source_directory, direntry.name, modified=relative_pathname in result.overwritten)
result.files_written.append(relative_pathname)
elif direntry.is_symlink():
- if self._check_replacement(direntry.name, path_prefix, result):
+ if self._check_replacement(direntry.name, relative_pathname, result):
self._copy_link_from_filesystem(source_directory, direntry.name)
result.files_written.append(relative_pathname)
@@ -303,18 +300,43 @@
is_dir = entry.type == _FileType.DIRECTORY
if is_dir:
- src_subdir = source_directory.descend(name)
+ create_subdir = name not in self.index
- try:
- create_subdir = name not in self.index
- dest_subdir = self.descend(name, create=create_subdir)
- except VirtualDirectoryError:
- filetype = self.index[name].type
- raise VirtualDirectoryError('Destination is a {}, not a directory: /{}'
- .format(filetype, relative_pathname))
+ if create_subdir and not filter_callback:
+ # If subdirectory does not exist yet and there is no filter,
+ # we can import the whole source directory by digest instead
+ # of importing each directory entry individually.
+ subdir_digest = entry.get_digest()
+ dest_entry = IndexEntry(name, _FileType.DIRECTORY, digest=subdir_digest)
+ self.index[name] = dest_entry
+ self.__invalidate_digest()
- dest_subdir._partial_import_cas_into_cas(src_subdir, filter_callback,
- path_prefix=relative_pathname, result=result)
+ # However, we still need to iterate over the directory entries
+ # to fill in `result.files_written`.
+
+ # Use source subdirectory object if it already exists,
+ # otherwise create object for destination subdirectory.
+ # This is based on the assumption that the destination
+ # subdirectory is more likely to be modified later on
+ # (e.g., by further import_files() calls).
+ if entry.buildstream_object:
+ subdir = entry.buildstream_object
+ else:
+ subdir = dest_entry.get_directory(self)
+
+ subdir.__add_files_to_result(path_prefix=relative_pathname, result=result)
+ else:
+ src_subdir = source_directory.descend(name)
+
+ try:
+ dest_subdir = self.descend(name, create=create_subdir)
+ except VirtualDirectoryError:
+ filetype = self.index[name].type
+ raise VirtualDirectoryError('Destination is a {}, not a directory: /{}'
+ .format(filetype, relative_pathname))
+
+ dest_subdir._partial_import_cas_into_cas(src_subdir, filter_callback,
+ path_prefix=relative_pathname, result=result)
if filter_callback and not filter_callback(relative_pathname):
if is_dir and create_subdir and dest_subdir.is_empty():
@@ -325,7 +347,7 @@
continue
if not is_dir:
- if self._check_replacement(name, path_prefix, result):
+ if self._check_replacement(name, relative_pathname, result):
if entry.type == _FileType.REGULAR_FILE:
self.index[name] = IndexEntry(name, _FileType.REGULAR_FILE,
digest=entry.digest,
@@ -556,13 +578,13 @@
return self.__digest
def _objpath(self, path):
- subdir = self.descend(path[:-1])
+ subdir = self.descend(*path[:-1])
entry = subdir.index[path[-1]]
return self.cas_cache.objpath(entry.digest)
def _exists(self, path):
try:
- subdir = self.descend(path[:-1])
+ subdir = self.descend(*path[:-1])
return path[-1] in subdir.index
except VirtualDirectoryError:
return False
@@ -572,3 +594,14 @@
self.__digest = None
if self.parent:
self.parent.__invalidate_digest()
+
+ def __add_files_to_result(self, *, path_prefix="", result):
+ for name, entry in self.index.items():
+ # The destination filename, relative to the root where the import started
+ relative_pathname = os.path.join(path_prefix, name)
+
+ if entry.type == _FileType.DIRECTORY:
+ subdir = self.descend(name)
+ subdir.__add_files_to_result(path_prefix=relative_pathname, result=result)
+ else:
+ result.files_written.append(relative_pathname)
diff --git a/buildstream/storage/_filebaseddirectory.py b/buildstream/storage/_filebaseddirectory.py
index 4b0fd91..7422003 100644
--- a/buildstream/storage/_filebaseddirectory.py
+++ b/buildstream/storage/_filebaseddirectory.py
@@ -46,35 +46,32 @@
def __init__(self, external_directory=None):
self.external_directory = external_directory
- def descend(self, subdirectory_spec, create=False):
+ def descend(self, *paths, create=False):
""" See superclass Directory for arguments """
- # It's very common to send a directory name instead of a list and this causes
- # bizarre errors, so check for it here
- if not isinstance(subdirectory_spec, list):
- subdirectory_spec = [subdirectory_spec]
- # Because of the way split works, it's common to get a list which begins with
- # an empty string. Detect these and remove them.
- while subdirectory_spec and subdirectory_spec[0] == "":
- subdirectory_spec.pop(0)
+ current_dir = self
- if not subdirectory_spec:
- return self
+ for path in paths:
+ # Skip empty path segments
+ if not path:
+ continue
- new_path = os.path.join(self.external_directory, subdirectory_spec[0])
- try:
- st = os.lstat(new_path)
- if not stat.S_ISDIR(st.st_mode):
- raise VirtualDirectoryError("Cannot descend into '{}': '{}' is not a directory"
- .format(subdirectory_spec[0], new_path))
- except FileNotFoundError:
- if create:
- os.mkdir(new_path)
- else:
- raise VirtualDirectoryError("Cannot descend into '{}': '{}' does not exist"
- .format(subdirectory_spec[0], new_path))
+ new_path = os.path.join(current_dir.external_directory, path)
+ try:
+ st = os.lstat(new_path)
+ if not stat.S_ISDIR(st.st_mode):
+ raise VirtualDirectoryError("Cannot descend into '{}': '{}' is not a directory"
+ .format(path, new_path))
+ except FileNotFoundError:
+ if create:
+ os.mkdir(new_path)
+ else:
+ raise VirtualDirectoryError("Cannot descend into '{}': '{}' does not exist"
+ .format(path, new_path))
- return FileBasedDirectory(new_path).descend(subdirectory_spec[1:], create)
+ current_dir = FileBasedDirectory(new_path)
+
+ return current_dir
def import_files(self, external_pathspec, *,
filter_callback=None,
@@ -160,7 +157,7 @@
tf.addfile(tarinfo, f)
elif tarinfo.isdir():
tf.addfile(tarinfo)
- self.descend(filename.split(os.path.sep)).export_to_tar(tf, arcname, mtime)
+ self.descend(*filename.split(os.path.sep)).export_to_tar(tf, arcname, mtime)
else:
tf.addfile(tarinfo)
diff --git a/buildstream/storage/directory.py b/buildstream/storage/directory.py
index 70054f7..bad818f 100644
--- a/buildstream/storage/directory.py
+++ b/buildstream/storage/directory.py
@@ -52,13 +52,12 @@
def __init__(self, external_directory=None):
raise NotImplementedError()
- def descend(self, subdirectory_spec, create=False):
+ def descend(self, *paths, create=False):
"""Descend one or more levels of directory hierarchy and return a new
Directory object for that directory.
Args:
- subdirectory_spec (list of str): A list of strings which are all directory
- names.
+ *paths (str): A list of strings which are all directory names.
create (boolean): If this is true, the directories will be created if
they don't already exist.
