diff options
author | Todd Gamblin <tgamblin@llnl.gov> | 2017-10-12 09:52:38 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2017-10-12 09:52:38 -0700 |
commit | 65b38764aedf56192568f5d49eb9a9c87257761b (patch) | |
tree | 4041225afc921ffe1ca41230903b9370a5e91273 /lib | |
parent | db149876a42ec4cc8db9e488d90d71c7d811a7a6 (diff) | |
download | spack-65b38764aedf56192568f5d49eb9a9c87257761b.tar.gz spack-65b38764aedf56192568f5d49eb9a9c87257761b.tar.bz2 spack-65b38764aedf56192568f5d49eb9a9c87257761b.tar.xz spack-65b38764aedf56192568f5d49eb9a9c87257761b.zip |
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
Diffstat (limited to 'lib')
-rw-r--r-- | lib/spack/spack/concretize.py | 6 | ||||
-rw-r--r-- | lib/spack/spack/provider_index.py | 4 | ||||
-rw-r--r-- | lib/spack/spack/repository.py | 25 | ||||
-rw-r--r-- | lib/spack/spack/variant.py | 5 |
4 files changed, 23 insertions, 17 deletions
diff --git a/lib/spack/spack/concretize.py b/lib/spack/spack/concretize.py index 1aed7d8eb1..10a3705083 100644 --- a/lib/spack/spack/concretize.py +++ b/lib/spack/spack/concretize.py @@ -146,8 +146,8 @@ class DefaultConcretizer(object): return False # List of versions we could consider, in sorted order - pkg = spec.package - usable = [v for v in pkg.versions + pkg_versions = spec.package_class.versions + usable = [v for v in pkg_versions if any(v.satisfies(sv) for sv in spec.versions)] yaml_prefs = PackagePrefs(spec.name, 'version') @@ -165,7 +165,7 @@ class DefaultConcretizer(object): -yaml_prefs(v), # 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. # Avoid @develop version, which would otherwise be the "largest" diff --git a/lib/spack/spack/provider_index.py b/lib/spack/spack/provider_index.py index 054f1ca807..15cb3e785e 100644 --- a/lib/spack/spack/provider_index.py +++ b/lib/spack/spack/provider_index.py @@ -97,8 +97,8 @@ class ProviderIndex(object): assert(not spec.virtual) - pkg = spec.package - for provided_spec, provider_specs in iteritems(pkg.provided): + pkg_provided = spec.package_class.provided + for provided_spec, provider_specs in iteritems(pkg_provided): for provider_spec in provider_specs: # TODO: fix this comment. # We want satisfaction other than flags diff --git a/lib/spack/spack/repository.py b/lib/spack/spack/repository.py index 0a43cf8af1..ba33e24069 100644 --- a/lib/spack/spack/repository.py +++ b/lib/spack/spack/repository.py @@ -537,20 +537,27 @@ class RepoPath(object): sys.modules[fullname] = module return module - @_autospec def repo_for_pkg(self, spec): """Given a spec, get the repository for its package.""" - # If the spec already has a namespace, then return the - # corresponding repo if we know about it. - if spec.namespace: - fullspace = '%s.%s' % (self.super_namespace, spec.namespace) - if fullspace not in self.by_namespace: - raise UnknownNamespaceError(spec.namespace) - return self.by_namespace[fullspace] + # 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 + # corresponding repo if we know about it. + if spec.namespace: + fullspace = '%s.%s' % (self.super_namespace, spec.namespace) + if fullspace not in self.by_namespace: + raise UnknownNamespaceError(spec.namespace) + 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. for repo in self.repos: - if spec.name in repo: + if name in repo: return repo # If the package isn't in any repo, return the one with diff --git a/lib/spack/spack/variant.py b/lib/spack/spack/variant.py index 3f9ede1047..4553bf53e4 100644 --- a/lib/spack/spack/variant.py +++ b/lib/spack/spack/variant.py @@ -592,10 +592,9 @@ def substitute_abstract_variants(spec): spec: spec on which to operate the substitution """ for name, v in spec.variants.items(): - pkg_cls = type(spec.package) - pkg_variant = spec.package.variants[name] + pkg_variant = spec.package_class.variants[name] 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) |