summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPeter Scheibel <scheibel1@llnl.gov>2019-08-19 11:24:05 -0700
committerTamara Dahlgren <35777542+tldahlgren@users.noreply.github.com>2019-08-19 11:24:05 -0700
commit31ff791180614c03f2727349bfb74d9ecfdb7ef7 (patch)
treecfd17bf27138e6594dee0263c5bb0edf063369d1
parent0ea6e0f8176db1ec28f1d65aa26c932f1ed055e7 (diff)
downloadspack-31ff791180614c03f2727349bfb74d9ecfdb7ef7.tar.gz
spack-31ff791180614c03f2727349bfb74d9ecfdb7ef7.tar.bz2
spack-31ff791180614c03f2727349bfb74d9ecfdb7ef7.tar.xz
spack-31ff791180614c03f2727349bfb74d9ecfdb7ef7.zip
features: Update compiler caching (#7675)
Compiler caching was using the `id()` function to refer to configuration dictionary objects. If these objects are garbage-collected, this can produce incorrect results (false positive cache hits). This change replaces `id()` with an object that keeps a reference to the config dictionary so that it is not garbage-collected.
-rw-r--r--lib/spack/spack/compilers/__init__.py88
-rw-r--r--lib/spack/spack/test/compilers.py6
2 files changed, 60 insertions, 34 deletions
diff --git a/lib/spack/spack/compilers/__init__.py b/lib/spack/spack/compilers/__init__.py
index 5adfd17b1a..20f40eef09 100644
--- a/lib/spack/spack/compilers/__init__.py
+++ b/lib/spack/spack/compilers/__init__.py
@@ -278,7 +278,7 @@ def all_compilers(scope=None):
compilers = list()
for items in config:
items = items['compiler']
- compilers.append(compiler_from_config_entry(items))
+ compilers.append(_compiler_from_config_entry(items))
return compilers
@@ -305,41 +305,69 @@ def compilers_for_arch(arch_spec, scope=None):
return list(get_compilers(config, arch_spec=arch_spec))
-def compiler_from_config_entry(items):
- config_id = id(items)
- compiler = _compiler_cache.get(config_id, None)
+class CacheReference(object):
+ """This acts as a hashable reference to any object (regardless of whether
+ the object itself is hashable) and also prevents the object from being
+ garbage-collected (so if two CacheReference objects are equal, they
+ will refer to the same object, since it will not have been gc'ed since
+ the creation of the first CacheReference).
+ """
+ def __init__(self, val):
+ self.val = val
+ self.id = id(val)
+
+ def __hash__(self):
+ return self.id
+
+ def __eq__(self, other):
+ return isinstance(other, CacheReference) and self.id == other.id
- if compiler is None:
- cspec = spack.spec.CompilerSpec(items['spec'])
- os = items.get('operating_system', None)
- target = items.get('target', None)
- if not ('paths' in items and
- all(n in items['paths'] for n in _path_instance_vars)):
- raise InvalidCompilerConfigurationError(cspec)
+def compiler_from_dict(items):
+ cspec = spack.spec.CompilerSpec(items['spec'])
+ os = items.get('operating_system', None)
+ target = items.get('target', None)
- cls = class_for_compiler_name(cspec.name)
+ if not ('paths' in items and
+ all(n in items['paths'] for n in _path_instance_vars)):
+ raise InvalidCompilerConfigurationError(cspec)
- compiler_paths = []
- for c in _path_instance_vars:
- compiler_path = items['paths'][c]
- if compiler_path != 'None':
- compiler_paths.append(compiler_path)
- else:
- compiler_paths.append(None)
+ cls = class_for_compiler_name(cspec.name)
- mods = items.get('modules')
- if mods == 'None':
- mods = []
+ compiler_paths = []
+ for c in _path_instance_vars:
+ compiler_path = items['paths'][c]
+ if compiler_path != 'None':
+ compiler_paths.append(compiler_path)
+ else:
+ compiler_paths.append(None)
- alias = items.get('alias', None)
- compiler_flags = items.get('flags', {})
- environment = items.get('environment', {})
- extra_rpaths = items.get('extra_rpaths', [])
+ mods = items.get('modules')
+ if mods == 'None':
+ mods = []
- compiler = cls(cspec, os, target, compiler_paths, mods, alias,
- environment, extra_rpaths, **compiler_flags)
- _compiler_cache[id(items)] = compiler
+ alias = items.get('alias', None)
+ compiler_flags = items.get('flags', {})
+ environment = items.get('environment', {})
+ extra_rpaths = items.get('extra_rpaths', [])
+
+ return cls(cspec, os, target, compiler_paths, mods, alias,
+ environment, extra_rpaths, **compiler_flags)
+
+
+def _compiler_from_config_entry(items):
+ """Note this is intended for internal use only. To avoid re-parsing
+ the same config dictionary this keeps track of its location in
+ memory. If you provide the same dictionary twice it will return
+ the same Compiler object (regardless of whether the dictionary
+ entries have changed).
+ """
+ config_id = CacheReference(items)
+ compiler = _compiler_cache.get(config_id, None)
+
+ if compiler is None:
+ compiler = compiler_from_dict(items)
+ _compiler_cache[config_id] = compiler
return compiler
@@ -367,7 +395,7 @@ def get_compilers(config, cspec=None, arch_spec=None):
target != 'any'):
continue
- compilers.append(compiler_from_config_entry(items))
+ compilers.append(_compiler_from_config_entry(items))
return compilers
diff --git a/lib/spack/spack/test/compilers.py b/lib/spack/spack/test/compilers.py
index 00a0fab41c..f34b9ceb5a 100644
--- a/lib/spack/spack/test/compilers.py
+++ b/lib/spack/spack/test/compilers.py
@@ -129,7 +129,7 @@ def test_compiler_flags_from_config_are_grouped():
'modules': None
}
- compiler = compilers.compiler_from_config_entry(compiler_entry)
+ compiler = compilers.compiler_from_dict(compiler_entry)
assert any(x == '-foo-flag foo-val' for x in compiler.flags['cflags'])
@@ -179,9 +179,7 @@ def flag_value(flag, spec):
else:
compiler_entry = copy(default_compiler_entry)
compiler_entry['spec'] = spec
- # Disable faulty id()-based cache (issue #7647).
- compilers._compiler_cache = {}
- compiler = compilers.compiler_from_config_entry(compiler_entry)
+ compiler = compilers.compiler_from_dict(compiler_entry)
return getattr(compiler, flag)