From 8e8235043d6f4605f338761233178404840baae5 Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Sun, 29 Dec 2019 00:12:14 -0800 Subject: package_prefs: move class-level cache to PackagePref instance `PackagePrefs` has had a class-level cache of data from `packages.yaml` for a long time, but it complicates testing and leads to subtle errors, especially now that we frequently manipulate custom config scopes and environments. Moving the cache to instance-level doesn't slow down concretization or the test suite, and it just caches for the life of a `PackagePrefs` instance (i.e., for a single cocncretization) so we don't need to worry about global state anymore. - [x] Remove class-level caches from `PackagePrefs` - [x] Add a cached _spec_order object on each `PackagePrefs` instance - [x] Remove all calls to `PackagePrefs.clear_caches()` --- lib/spack/llnl/util/lang.py | 6 ---- lib/spack/spack/package_prefs.py | 42 +++++++------------------- lib/spack/spack/test/cmd/env.py | 10 ------ lib/spack/spack/test/concretize_preferences.py | 3 -- lib/spack/spack/test/conftest.py | 7 ----- 5 files changed, 11 insertions(+), 57 deletions(-) (limited to 'lib') diff --git a/lib/spack/llnl/util/lang.py b/lib/spack/llnl/util/lang.py index bdb551dcad..2fbef0fd46 100644 --- a/lib/spack/llnl/util/lang.py +++ b/lib/spack/llnl/util/lang.py @@ -19,12 +19,6 @@ import sys ignore_modules = [r'^\.#', '~$'] -class classproperty(property): - """classproperty decorator: like property but for classmethods.""" - def __get__(self, cls, owner): - return self.fget.__get__(None, owner)() - - def index_by(objects, *funcs): """Create a hierarchy of dictionaries by splitting the supplied set of objects on unique values of the supplied functions. diff --git a/lib/spack/spack/package_prefs.py b/lib/spack/spack/package_prefs.py index e73ce7c947..56e2e53a83 100644 --- a/lib/spack/spack/package_prefs.py +++ b/lib/spack/spack/package_prefs.py @@ -8,8 +8,6 @@ import stat from six import string_types from six import iteritems -from llnl.util.lang import classproperty - import spack.repo import spack.error from spack.util.path import canonicalize_path @@ -75,14 +73,13 @@ class PackagePrefs(object): provider_spec_list.sort(key=kf) """ - _packages_config_cache = None - _spec_cache = {} - def __init__(self, pkgname, component, vpkg=None): self.pkgname = pkgname self.component = component self.vpkg = vpkg + self._spec_order = None + def __call__(self, spec): """Return a key object (an index) that can be used to sort spec. @@ -90,8 +87,10 @@ class PackagePrefs(object): this function as Python's sort functions already ensure that the key function is called at most once per sorted element. """ - spec_order = self._specs_for_pkg( - self.pkgname, self.component, self.vpkg) + if self._spec_order is None: + self._spec_order = self._specs_for_pkg( + self.pkgname, self.component, self.vpkg) + spec_order = self._spec_order # integer is the index of the first spec in order that satisfies # spec, or it's a number larger than any position in the order. @@ -107,13 +106,6 @@ class PackagePrefs(object): match_index -= 0.5 return match_index - @classproperty - @classmethod - def _packages_config(cls): - if cls._packages_config_cache is None: - cls._packages_config_cache = get_packages_config() - return cls._packages_config_cache - @classmethod def order_for_package(cls, pkgname, component, vpkg=None, all=True): """Given a package name, sort component (e.g, version, compiler, ...), @@ -124,7 +116,7 @@ class PackagePrefs(object): pkglist.append('all') for pkg in pkglist: - pkg_entry = cls._packages_config.get(pkg) + pkg_entry = get_packages_config().get(pkg) if not pkg_entry: continue @@ -150,21 +142,9 @@ class PackagePrefs(object): return a list of CompilerSpecs, VersionLists, or Specs for that sorting list. """ - key = (pkgname, component, vpkg) - - specs = cls._spec_cache.get(key) - if specs is None: - pkglist = cls.order_for_package(pkgname, component, vpkg) - spec_type = _spec_type(component) - specs = [spec_type(s) for s in pkglist] - cls._spec_cache[key] = specs - - return specs - - @classmethod - def clear_caches(cls): - cls._packages_config_cache = None - cls._spec_cache = {} + pkglist = cls.order_for_package(pkgname, component, vpkg) + spec_type = _spec_type(component) + return [spec_type(s) for s in pkglist] @classmethod def has_preferred_providers(cls, pkgname, vpkg): @@ -180,7 +160,7 @@ class PackagePrefs(object): def preferred_variants(cls, pkg_name): """Return a VariantMap of preferred variants/values for a spec.""" for pkg in (pkg_name, 'all'): - variants = cls._packages_config.get(pkg, {}).get('variants', '') + variants = get_packages_config().get(pkg, {}).get('variants', '') if variants: break diff --git a/lib/spack/spack/test/cmd/env.py b/lib/spack/spack/test/cmd/env.py index 6de1546e09..654611565e 100644 --- a/lib/spack/spack/test/cmd/env.py +++ b/lib/spack/spack/test/cmd/env.py @@ -403,8 +403,6 @@ env: mpileaks: version: [2.2] """ - spack.package_prefs.PackagePrefs.clear_caches() - _env_create('test', StringIO(test_config)) e = ev.read('test') @@ -423,8 +421,6 @@ env: specs: - mpileaks """ - spack.package_prefs.PackagePrefs.clear_caches() - _env_create('test', StringIO(test_config)) e = ev.read('test') @@ -452,7 +448,6 @@ env: - mpileaks """ % config_scope_path - spack.package_prefs.PackagePrefs.clear_caches() _env_create('test', StringIO(test_config)) e = ev.read('test') @@ -483,9 +478,6 @@ env: specs: - mpileaks """ - - spack.package_prefs.PackagePrefs.clear_caches() - _env_create('test', StringIO(test_config)) e = ev.read('test') @@ -519,8 +511,6 @@ env: specs: - mpileaks """ - spack.package_prefs.PackagePrefs.clear_caches() - _env_create('test', StringIO(test_config)) e = ev.read('test') diff --git a/lib/spack/spack/test/concretize_preferences.py b/lib/spack/spack/test/concretize_preferences.py index e5fa4a851d..e3055192cd 100644 --- a/lib/spack/spack/test/concretize_preferences.py +++ b/lib/spack/spack/test/concretize_preferences.py @@ -23,7 +23,6 @@ def concretize_scope(config, tmpdir): yield config.pop_scope() - spack.package_prefs.PackagePrefs.clear_caches() spack.repo.path._provider_index = None @@ -60,7 +59,6 @@ def update_packages(pkgname, section, value): """Update config and reread package list""" conf = {pkgname: {section: value}} spack.config.set('packages', conf, scope='concretize') - spack.package_prefs.PackagePrefs.clear_caches() def assert_variant_values(spec, **variants): @@ -204,7 +202,6 @@ all: spack.config.set('packages', conf, scope='concretize') # should be no error for 'all': - spack.package_prefs.PackagePrefs.clear_caches() spack.package_prefs.get_packages_config() def test_external_mpi(self): diff --git a/lib/spack/spack/test/conftest.py b/lib/spack/spack/test/conftest.py index 3ac4d893af..b23c53c20e 100644 --- a/lib/spack/spack/test/conftest.py +++ b/lib/spack/spack/test/conftest.py @@ -349,8 +349,6 @@ def configuration_dir(tmpdir_factory, linux_os): def config(configuration_dir): """Hooks the mock configuration files into spack.config""" # Set up a mock config scope - spack.package_prefs.PackagePrefs.clear_caches() - real_configuration = spack.config.config defaults = spack.config.InternalConfigScope( @@ -367,14 +365,11 @@ def config(configuration_dir): yield spack.config.config spack.config.config = real_configuration - spack.package_prefs.PackagePrefs.clear_caches() @pytest.fixture(scope='function') def mutable_config(tmpdir_factory, configuration_dir, monkeypatch): """Like config, but tests can modify the configuration.""" - spack.package_prefs.PackagePrefs.clear_caches() - mutable_dir = tmpdir_factory.mktemp('mutable_config').join('tmp') configuration_dir.copy(mutable_dir) @@ -389,8 +384,6 @@ def mutable_config(tmpdir_factory, configuration_dir, monkeypatch): yield spack.config.config - spack.package_prefs.PackagePrefs.clear_caches() - @pytest.fixture() def mock_config(tmpdir): -- cgit v1.2.3-60-g2f50