Speed up concretization (#5716)

This isn't a rework of the concretizer but it speeds things up a LOT.

The main culprits were:
  1. Variant code, `provider_index`, and `concretize.py` were calling
     `spec.package` when they could use `spec.package_class`
    - `spec.package` looks up a package instance by `Spec`, which requires a
      (fast-ish but not that fast) DAG compare.
    - `spec.package_class` just looks up the package's class by name, and you
        should use this when all you need is metadata (most of the time).
    - not really clear that the current way packages are looked up is
      necessary -- we can consider refactoring that in the future.

  2. `Repository.repo_for_pkg` parses a `str` argument into a `Spec` when
     called with one, via `@_autospec`, but this is not needed.
     - Add some faster code to handle strings directly and avoid parsing

This speeds up concretization 3-9x in my limited tests.  Still not super
fast but much more bearable:

Before:
  - `spack spec xsdk` took 33.6s
  - `spack spec dealii` took 1m39s

After:
  - `spack spec xsdk` takes 6.8s
  - `spack spec dealii` takes 10.8s
This commit is contained in:
Todd Gamblin 2017-10-12 09:52:38 -07:00 committed by GitHub
parent db149876a4
commit 65b38764ae
4 changed files with 23 additions and 17 deletions

View File

@ -146,8 +146,8 @@ def concretize_version(self, spec):
return False return False
# List of versions we could consider, in sorted order # List of versions we could consider, in sorted order
pkg = spec.package pkg_versions = spec.package_class.versions
usable = [v for v in pkg.versions usable = [v for v in pkg_versions
if any(v.satisfies(sv) for sv in spec.versions)] if any(v.satisfies(sv) for sv in spec.versions)]
yaml_prefs = PackagePrefs(spec.name, 'version') yaml_prefs = PackagePrefs(spec.name, 'version')
@ -165,7 +165,7 @@ def concretize_version(self, spec):
-yaml_prefs(v), -yaml_prefs(v),
# The preferred=True flag (packages or packages.yaml or both?) # The preferred=True flag (packages or packages.yaml or both?)
pkg.versions.get(Version(v)).get('preferred', False), pkg_versions.get(Version(v)).get('preferred', False),
# ------- Regular case: use latest non-develop version by default. # ------- Regular case: use latest non-develop version by default.
# Avoid @develop version, which would otherwise be the "largest" # Avoid @develop version, which would otherwise be the "largest"

View File

@ -97,8 +97,8 @@ def update(self, spec):
assert(not spec.virtual) assert(not spec.virtual)
pkg = spec.package pkg_provided = spec.package_class.provided
for provided_spec, provider_specs in iteritems(pkg.provided): for provided_spec, provider_specs in iteritems(pkg_provided):
for provider_spec in provider_specs: for provider_spec in provider_specs:
# TODO: fix this comment. # TODO: fix this comment.
# We want satisfaction other than flags # We want satisfaction other than flags

View File

@ -537,9 +537,11 @@ def load_module(self, fullname):
sys.modules[fullname] = module sys.modules[fullname] = module
return module return module
@_autospec
def repo_for_pkg(self, spec): def repo_for_pkg(self, spec):
"""Given a spec, get the repository for its package.""" """Given a spec, get the repository for its package."""
# We don't @_autospec this function b/c it's called very frequently
# and we want to avoid parsing str's into Specs unnecessarily.
if isinstance(spec, spack.spec.Spec):
# If the spec already has a namespace, then return the # If the spec already has a namespace, then return the
# corresponding repo if we know about it. # corresponding repo if we know about it.
if spec.namespace: if spec.namespace:
@ -547,10 +549,15 @@ def repo_for_pkg(self, spec):
if fullspace not in self.by_namespace: if fullspace not in self.by_namespace:
raise UnknownNamespaceError(spec.namespace) raise UnknownNamespaceError(spec.namespace)
return self.by_namespace[fullspace] return self.by_namespace[fullspace]
name = spec.name
else:
# handle strings directly for speed instead of @_autospec'ing
name = spec
# If there's no namespace, search in the RepoPath. # If there's no namespace, search in the RepoPath.
for repo in self.repos: for repo in self.repos:
if spec.name in repo: if name in repo:
return repo return repo
# If the package isn't in any repo, return the one with # If the package isn't in any repo, return the one with

View File

@ -592,10 +592,9 @@ def substitute_abstract_variants(spec):
spec: spec on which to operate the substitution spec: spec on which to operate the substitution
""" """
for name, v in spec.variants.items(): for name, v in spec.variants.items():
pkg_cls = type(spec.package) pkg_variant = spec.package_class.variants[name]
pkg_variant = spec.package.variants[name]
new_variant = pkg_variant.make_variant(v._original_value) new_variant = pkg_variant.make_variant(v._original_value)
pkg_variant.validate_or_raise(new_variant, pkg_cls) pkg_variant.validate_or_raise(new_variant, spec.package_class)
spec.variants.substitute(new_variant) spec.variants.substitute(new_variant)