[Notes] [Git][BuildStream/buildstream][tpollard/896] 3 commits: contribu

Tom Pollard pushed to branch tpollard/896 at BuildStream / buildstream

Commits:

429c2834

by Angelos Evripiotis at 2019-02-11T11:22:53Z

contributing: snakeviz replaces pyflame+flamegraph

Replace the instructions for pyflame+flamegraph with simpler ones for
snakeviz. For our general use-case this seems to be easier and better.
Usage of this tool was demonstrated at the 2019 BuildStream Gathering in
January by Daniel Silverstone, when presenting the aggregate results of
profiling on many target environments.

Here is the relevant mailing list thread:

"Profiling before the gathering"
https://mail.gnome.org/archives/buildstream-list/2019-January/msg00057.html

3ee12ab5

by Tom Pollard at 2019-02-11T11:23:49Z

Add cli main & user conf option for 'cache-buildtrees' context

_context.py: Add cache_buildtrees global user context, the default
of which is set to by default to True via the addition of
cache-buildtrees to userconfig.yaml cache group.

app.py & cli.py: Add --cache-buildtrees/--no-cache-buildtrees as
a bst main option, which when passed can override the default or
user defined context for cache_buildtrees.

tests/completions/completions.py: Update for the added flag.

1f821d7c

by Tom Pollard at 2019-02-11T11:58:07Z

Provide configuration for the optional creation of buildtrees

Artifacts can cached explicitly with an empty `build tree` when
built via the cli main options or user config. Default behaviour
is to still create and cache expected buildtrees.

element.py: _cache_artifact() Check if context for cache_buildtrees
has been set to False, if so skip attempting to export the
build-root. Element types without a build-root are cached with an
empty buildtree regardless. Update _stage_sources_at() to warn the
user that the buildtree import is empty.

tests/integration: Add test to artifact.py for the optional caching
of buildtree content from bst build. Rename build-tree.py to
shellbuildtrees.py to reflect included test cases, add test for
empty buildtree warning.

NEWS: Add entry for new option.

10 changed files:

CONTRIBUTING.rst
NEWS
buildstream/_context.py
buildstream/_frontend/app.py
buildstream/_frontend/cli.py
buildstream/data/userconfig.yaml
buildstream/element.py
tests/frontend/completions.py
tests/integration/artifact.py
tests/integration/build-tree.py → tests/integration/shellbuildtrees.py

Changes:

CONTRIBUTING.rst

@@ -1707,26 +1707,13 @@ You can then analyze the results interactively using the 'pstats' module:
  For more detailed documentation of cProfile and 'pstats', see:
  https://docs.python.org/3/library/profile.html.
 -For a richer visualisation of the callstack you can try `Pyflame
 -<https://github.com/uber/pyflame>`_. Once you have followed the instructions in
 -Pyflame's README to install the tool, you can profile `bst` commands as in the
 -following example:
 +For a richer and interactive visualisation of the `.cprofile` files, you can
 +try `snakeviz <http://jiffyclub.github.io/snakeviz/#interpreting-results>`_.
 +You can install it with `pip install snakeviz`. Here is an example invocation:
 -    pyflame --output bst.flame --trace bst --help
+-
 -You may see an `Unexpected ptrace(2) exception:` error. Note that the `bst`
 -operation will continue running in the background in this case, you will need
 -to wait for it to complete or kill it. Once this is done, rerun the above
 -command which appears to fix the issue.
+-
 -Once you have output from pyflame, you can use the ``flamegraph.pl`` script
 -from the `Flamegraph project <https://github.com/brendangregg/FlameGraph>`_
 -to generate an .svg image:
+-
 -    ./flamegraph.pl bst.flame > bst-flamegraph.svg
+-
 -The generated SVG file can then be viewed in your preferred web browser.
 +    snakeviz bst.cprofile
 +It will then start a webserver and launch a browser to the relevant page.
  Profiling specific parts of BuildStream with BST_PROFILE
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

NEWS

@@ -126,6 +126,13 @@ buildstream 1.3.1
      Providing a remote will limit build's pull/push remote actions to the given
      remote specifically, ignoring those defined via user or project configuration.
 +  o Artifacts can now be cached explicitly with an empty `build tree` when built.
 +    Element types without a build-root were already cached with an empty build tree
 +    directory, this can now be extended to all artifacts to save on cache overheads.
 +    The cli main option 'no-cache-buildtrees' or the user configuration cache group
 +    option 'cache-buildtrees = False' can be set to enable this behaviour. Note, as
 +    the cache-key for the artifact is independant of the cached build tree input it will
 +    remain unaltered, however the availbility of the build tree content may differ.
  =================
  buildstream 1.1.5

buildstream/_context.py

@@ -121,6 +121,9 @@ class Context():
          # Whether or not to attempt to pull build trees globally
          self.pull_buildtrees = None
 +        # Whether or not to cache build trees on artifact creation
 +        self.cache_buildtrees = None
++
          # Boolean, whether we double-check with the user that they meant to
          # close the workspace when they're using it to access the project.
          self.prompt_workspace_close_project_inaccessible = None
@@ -201,7 +204,7 @@ class Context():
          # our artifactdir - the artifactdir may not have been created
          # yet.
          cache = _yaml.node_get(defaults, Mapping, 'cache')
 -        _yaml.node_validate(cache, ['quota', 'pull-buildtrees'])
 +        _yaml.node_validate(cache, ['quota', 'pull-buildtrees', 'cache-buildtrees'])
          self.config_cache_quota = _yaml.node_get(cache, str, 'quota')
@@ -213,6 +216,9 @@ class Context():
          # Load pull build trees configuration
          self.pull_buildtrees = _yaml.node_get(cache, bool, 'pull-buildtrees')
 +        # Load cache build trees configuration
 +        self.cache_buildtrees = _yaml.node_get(cache, bool, 'cache-buildtrees')
++
          # Load logging config
          logging = _yaml.node_get(defaults, Mapping, 'logging')
          _yaml.node_validate(logging, [

buildstream/_frontend/app.py

@@ -183,7 +183,8 @@ class App():
              'builders': 'sched_builders',
              'pushers': 'sched_pushers',
              'network_retries': 'sched_network_retries',
 -            'pull_buildtrees': 'pull_buildtrees'
 +            'pull_buildtrees': 'pull_buildtrees',
 +            'cache_buildtrees': 'cache_buildtrees'
+         }
          for cli_option, context_attr in override_map.items():
              option_value = self._main_options.get(cli_option)

buildstream/_frontend/cli.py

@@ -252,6 +252,8 @@ def print_version(ctx, param, value):
                help="The mirror to fetch from first, before attempting other mirrors")
  @click.option('--pull-buildtrees', is_flag=True, default=None,
                help="Include an element's build tree when pulling remote element artifacts")
 +@click.option('--cache-buildtrees/--no-cache-buildtrees', default=None,
 +              help="Cache artifact build tree content on creation")
  @click.pass_context
  def cli(context, **kwargs):
      """Build and manipulate BuildStream projects

buildstream/data/userconfig.yaml


   # Whether to pull build trees when downloading element artifacts
   pull-buildtrees: False
 
+  # Whether to cache build trees on artifact creation
+  cache-buildtrees: True
+
 #
 #    Scheduler
 #

buildstream/element.py

@@ -1425,6 +1425,9 @@ class Element(Plugin):
              elif usebuildtree:
                  artifact_base, _ = self.__extract()
                  import_dir = os.path.join(artifact_base, 'buildtree')
 +                if not os.listdir(import_dir):
 +                    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)
              else:
                  # No workspace or cached buildtree, stage source directly
                  for source in self.sources():
@@ -1631,6 +1634,8 @@ class Element(Plugin):
                  # No collect directory existed
                  collectvdir = None
 +        context = self._get_context()
++
          # Create artifact directory structure
          assembledir = os.path.join(rootdir, 'artifact')
          filesdir = os.path.join(assembledir, 'files')
@@ -1648,20 +1653,27 @@ class Element(Plugin):
          if collect is not None and collectvdir is not None:
              collectvdir.export_files(filesdir, can_link=True)
 -        try:
 -            sandbox_vroot = sandbox.get_virtual_directory()
 -            sandbox_build_dir = sandbox_vroot.descend(
 -                self.get_variable('build-root').lstrip(os.sep).split(os.sep))
 -            # Hard link files from build-root dir to buildtreedir directory
 -            sandbox_build_dir.export_files(buildtreedir)
 -        except VirtualDirectoryError:
 -            # Directory could not be found. Pre-virtual
 -            # directory behaviour was to continue silently
 -            # if the directory could not be found.
 -            pass
 +        cache_buildtrees = context.cache_buildtrees
++
 +        # cache_buildtrees defaults to True, as such the
 +        # default behaviour is to attempt to cache them. Element
 +        # types without a build-root dir will be cached with an empty
 +        # buildtreedir regardless of this configuration.
 +        if cache_buildtrees:
 +            try:
 +                sandbox_vroot = sandbox.get_virtual_directory()
 +                sandbox_build_dir = sandbox_vroot.descend(
 +                    self.get_variable('build-root').lstrip(os.sep).split(os.sep))
 +                # Hard link files from build-root dir to buildtreedir directory
 +                sandbox_build_dir.export_files(buildtreedir)
 +            except VirtualDirectoryError:
 +                # Directory could not be found. Pre-virtual
 +                # directory behaviour was to continue silently
 +                # if the directory could not be found.
 +                pass
          # Copy build log
 -        log_filename = self._get_context().get_log_filename()
 +        log_filename = context.get_log_filename()
          self._build_log_path = os.path.join(logsdir, 'build.log')
          if log_filename:
              shutil.copyfile(log_filename, self._build_log_path)
@@ -1802,7 +1814,7 @@ class Element(Plugin):
              return True
          # Do not push elements that aren't cached, or that are cached with a dangling buildtree
 -        # artifact unless element type is expected to have an an empty buildtree directory
 +        # ref unless element type is expected to have an an empty buildtree directory
          if not self._cached_buildtree():
              return True
@@ -2004,6 +2016,8 @@ class Element(Plugin):
      # Returns:
      #     (bool): True if artifact cached with buildtree, False if
      #             element not cached or missing expected buildtree.
 +    #             Note this only confirms if a buildtree is present,
 +    #             not it's contents.
+     #
      def _cached_buildtree(self):
          context = self._get_context()

tests/frontend/completions.py

@@ -23,6 +23,7 @@ MAIN_OPTIONS = [
      "--builders ",
      "-c ",
      "-C ",
 +    "--cache-buildtrees ",
      "--colors ",
      "--config ",
      "--debug ",
@@ -33,6 +34,7 @@ MAIN_OPTIONS = [
      "--log-file ",
      "--message-lines ",
      "--network-retries ",
 +    "--no-cache-buildtrees ",
      "--no-colors ",
      "--no-debug ",
      "--no-interactive ",

tests/integration/artifact.py

@@ -20,9 +20,12 @@
  import os
  import pytest
 +import shutil
  from buildstream.plugintestutils import cli_integration as cli
+-
 +from tests.testutils import create_artifact_share
 +from tests.testutils.site import HAVE_SANDBOX
 +from buildstream._exceptions import ErrorDomain
  pytestmark = pytest.mark.integration
@@ -66,3 +69,106 @@ def test_artifact_log(cli, tmpdir, datafiles):
      assert result.exit_code == 0
      # The artifact is cached under both a strong key and a weak key
      assert (log + log) == result.output
++
++
 +# A test to capture the integration of the cachebuildtrees
 +# behaviour, which by default is to include the buildtree
 +# content of an element on caching.
 +@pytest.mark.integration
 +@pytest.mark.datafiles(DATA_DIR)
 +@pytest.mark.skipif(not HAVE_SANDBOX, reason='Only available with a functioning sandbox')
 +def test_cache_buildtrees(cli, tmpdir, datafiles):
 +    project = os.path.join(datafiles.dirname, datafiles.basename)
 +    element_name = 'autotools/amhello.bst'
++
 +    # Create artifact shares for pull & push testing
 +    with create_artifact_share(os.path.join(str(tmpdir), 'share1')) as share1,\
 +        create_artifact_share(os.path.join(str(tmpdir), 'share2')) as share2,\
 +        create_artifact_share(os.path.join(str(tmpdir), 'share3')) as share3:
 +        cli.configure({
 +            'artifacts': {'url': share1.repo, 'push': True},
 +            'artifactdir': os.path.join(str(tmpdir), 'artifacts')
 +        })
++
 +        # Build autotools element with cache-buildtrees set via the
 +        # cli. The artifact should be successfully pushed to the share1 remote
 +        # and cached locally with an 'empty' buildtree digest, as it's not a
 +        # dangling ref
 +        result = cli.run(project=project, args=['--no-cache-buildtrees', 'build', element_name])
 +        assert result.exit_code == 0
 +        assert cli.get_element_state(project, element_name) == 'cached'
 +        assert share1.has_artifact('test', element_name, cli.get_element_key(project, element_name))
++
 +        # The extracted buildtree dir should be empty, as we set the config
 +        # to not cache buildtrees
 +        cache_key = cli.get_element_key(project, element_name)
 +        elementdigest = share1.has_artifact('test', element_name, cache_key)
 +        buildtreedir = os.path.join(str(tmpdir), 'artifacts', 'extract', 'test', 'autotools-amhello',
 +                                    elementdigest.hash, 'buildtree')
 +        assert os.path.isdir(buildtreedir)
 +        assert not os.listdir(buildtreedir)
++
 +        # Delete the local cached artifacts, and assert the when pulled with --pull-buildtrees
 +        # that is was cached in share1 as expected with an empty buildtree dir
 +        shutil.rmtree(os.path.join(str(tmpdir), 'artifacts'))
 +        assert cli.get_element_state(project, element_name) != 'cached'
 +        result = cli.run(project=project, args=['--pull-buildtrees', 'artifact', 'pull', element_name])
 +        assert element_name in result.get_pulled_elements()
 +        assert os.path.isdir(buildtreedir)
 +        assert not os.listdir(buildtreedir)
 +        shutil.rmtree(os.path.join(str(tmpdir), 'artifacts'))
++
 +        # Assert that the default behaviour of pull to not include buildtrees on the artifact
 +        # in share1 which was purposely cached with an empty one behaves as expected. As such the
 +        # pulled artifact will have a dangling ref for the buildtree dir, regardless of content,
 +        # leading to no buildtreedir being extracted
 +        result = cli.run(project=project, args=['artifact', 'pull', element_name])
 +        assert element_name in result.get_pulled_elements()
 +        assert not os.path.isdir(buildtreedir)
 +        shutil.rmtree(os.path.join(str(tmpdir), 'artifacts'))
++
 +        # Repeat building the artifacts, this time with the default behaviour of caching buildtrees,
 +        # as such the buildtree dir should not be empty
 +        cli.configure({
 +            'artifacts': {'url': share2.repo, 'push': True},
 +            'artifactdir': os.path.join(str(tmpdir), 'artifacts')
 +        })
 +        result = cli.run(project=project, args=['build', element_name])
 +        assert result.exit_code == 0
 +        assert cli.get_element_state(project, element_name) == 'cached'
 +        assert share2.has_artifact('test', element_name, cli.get_element_key(project, element_name))
++
 +        # Cache key will be the same however the digest hash will have changed as expected, so reconstruct paths
 +        elementdigest = share2.has_artifact('test', element_name, cache_key)
 +        buildtreedir = os.path.join(str(tmpdir), 'artifacts', 'extract', 'test', 'autotools-amhello',
 +                                    elementdigest.hash, 'buildtree')
 +        assert os.path.isdir(buildtreedir)
 +        assert os.listdir(buildtreedir) is not None
++
 +        # Delete the local cached artifacts, and assert that when pulled with --pull-buildtrees
 +        # that it was cached in share2 as expected with a populated buildtree dir
 +        shutil.rmtree(os.path.join(str(tmpdir), 'artifacts'))
 +        assert cli.get_element_state(project, element_name) != 'cached'
 +        result = cli.run(project=project, args=['--pull-buildtrees', 'artifact', 'pull', element_name])
 +        assert element_name in result.get_pulled_elements()
 +        assert os.path.isdir(buildtreedir)
 +        assert os.listdir(buildtreedir) is not None
 +        shutil.rmtree(os.path.join(str(tmpdir), 'artifacts'))
++
 +        # Clarify that the user config option for cache-buildtrees works as the cli
 +        # main option does. Point to share3 which does not have the artifacts cached to force
 +        # a build
 +        cli.configure({
 +            'artifacts': {'url': share3.repo, 'push': True},
 +            'artifactdir': os.path.join(str(tmpdir), 'artifacts'),
 +            'cache': {'cache-buildtrees': False}
 +        })
 +        result = cli.run(project=project, args=['build', element_name])
 +        assert result.exit_code == 0
 +        assert cli.get_element_state(project, element_name) == 'cached'
 +        cache_key = cli.get_element_key(project, element_name)
 +        elementdigest = share3.has_artifact('test', element_name, cache_key)
 +        buildtreedir = os.path.join(str(tmpdir), 'artifacts', 'extract', 'test', 'autotools-amhello',
 +                                    elementdigest.hash, 'buildtree')
 +        assert os.path.isdir(buildtreedir)
 +        assert not os.listdir(buildtreedir)

tests/integration/build-tree.py → tests/integration/shellbuildtrees.py

@@ -52,6 +52,30 @@ def test_buildtree_staged_forced_true(cli_integration, tmpdir, datafiles):
      assert 'Hi' in res.output
 +@pytest.mark.datafiles(DATA_DIR)
 +@pytest.mark.skipif(not HAVE_SANDBOX, reason='Only available with a functioning sandbox')
 +def test_buildtree_staged_warn_empty(cli, tmpdir, datafiles):
 +    # Test that if we stage a cached and empty, we warn the user.
 +    project = os.path.join(datafiles.dirname, datafiles.basename)
 +    element_name = 'build-shell/buildtree.bst'
++
 +    # Switch to a temp artifact cache dir to ensure the artifact is rebuilt,
 +    # caching an empty buildtree
 +    cli.configure({
 +        'artifactdir': os.path.join(cli.directory, 'artifacts2')
 +    })
++
 +    res = cli.run(project=project, args=['--no-cache-buildtrees', 'build', element_name])
 +    res.assert_success()
++
 +    res = cli.run(project=project, args=[
 +        'shell', '--build', '--use-buildtree', 'always', element_name, '--', 'cat', 'test'
 +    ])
 +    res.assert_shell_error()
 +    assert "Artifact contains an empty buildtree" in res.stderr
 +    shutil.rmtree(os.path.join(cli.directory, 'artifacts2'))
++
++
  @pytest.mark.datafiles(DATA_DIR)
  @pytest.mark.skipif(not HAVE_SANDBOX, reason='Only available with a functioning sandbox')
  def test_buildtree_staged_if_available(cli_integration, tmpdir, datafiles):

...	...	@@ -41,6 +41,9 @@ cache:
41	41	# Whether to pull build trees when downloading element artifacts
42	42	pull-buildtrees: False
43	43
	44	+ # Whether to cache build trees on artifact creation
	45	+ cache-buildtrees: True
	46	+
44	47	#
45	48	# Scheduler
46	49	#

[Notes] [Git][BuildStream/buildstream][tpollard/896] 3 commits: contributing: snakeviz replaces pyflame+flamegraph

Tom Pollard pushed to branch tpollard/896 at BuildStream / buildstream

Commits:

10 changed files:

Changes: