Skip to content

Commit

Permalink
Speedup environment activation, part 2 (#25633)
Browse files Browse the repository at this point in the history
This is a direct followup to #13557 which caches additional attributes that were added in #24095 that are expensive to compute. I had to reopen #25556 in another PR to invalidate the GitLab CI cache, but see #25556 for prior discussion.

### Before

```console
$ time spack env activate .

real	2m13.037s
user	1m25.584s
sys	0m43.654s
$ time spack env view regenerate
==> Updating view at /Users/Adam/.spack/.spack-env/view

real	16m3.541s
user	10m28.892s
sys	4m57.816s
$ time spack env deactivate

real	2m30.974s
user	1m38.090s
sys	0m49.781s
```

### After
```console
$ time spack env activate .

real	0m8.937s
user	0m7.323s
sys	0m1.074s
$ time spack env view regenerate
==> Updating view at /Users/Adam/.spack/.spack-env/view

real	2m22.024s
user	1m44.739s
sys	0m30.717s
$ time spack env deactivate

real	0m10.398s
user	0m8.414s
sys	0m1.630s
```

Fixes #25555
Fixes #25541 

* Speedup environment activation, part 2
* Only query distutils a single time
* Fix KeyError bug
* Make vermin happy
* Manual memoize
* Add comment on cross-compiling
* Use platform-specific include directory
* Fix multiple bugs
* Fix python_inc discrepancy
* Fix import tests
  • Loading branch information
adamjstewart committed Aug 26, 2021
1 parent 9dab298 commit 6eb942c
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 113 deletions.
16 changes: 9 additions & 7 deletions lib/spack/spack/build_systems/python.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,10 @@ def import_modules(self):
list: list of strings of module names
"""
modules = []
root = self.spec['python'].package.get_python_lib(prefix=self.prefix)
root = os.path.join(
self.prefix,
self.spec['python'].package.config_vars['python_lib']['false']['false'],
)

# Some Python libraries are packages: collections of modules
# distributed in directories containing __init__.py files
Expand Down Expand Up @@ -252,12 +255,11 @@ def install_args(self, spec, prefix):
# Get all relative paths since we set the root to `prefix`
# We query the python with which these will be used for the lib and inc
# directories. This ensures we use `lib`/`lib64` as expected by python.
pure_site_packages_dir = spec['python'].package.get_python_lib(
plat_specific=False, prefix='')
plat_site_packages_dir = spec['python'].package.get_python_lib(
plat_specific=True, prefix='')
inc_dir = spec['python'].package.get_python_inc(
plat_specific=True, prefix='')
pure_site_packages_dir = spec['python'].package.config_vars[
'python_lib']['false']['false']
plat_site_packages_dir = spec['python'].package.config_vars[
'python_lib']['true']['false']
inc_dir = spec['python'].package.config_vars['python_inc']['true']

args += ['--root=%s' % prefix,
'--install-purelib=%s' % pure_site_packages_dir,
Expand Down
5 changes: 4 additions & 1 deletion lib/spack/spack/build_systems/sip.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,10 @@ def import_modules(self):
list: list of strings of module names
"""
modules = []
root = self.spec['python'].package.get_python_lib(prefix=self.prefix)
root = os.path.join(
self.prefix,
self.spec['python'].package.config_vars['python_lib']['false']['false'],
)

# Some Python libraries are packages: collections of modules
# distributed in directories containing __init__.py files
Expand Down
3 changes: 2 additions & 1 deletion var/spack/repos/builtin/packages/clingo/package.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ def cmake_python_hints(self):
current spec is the one found by CMake find_package(Python, ...)
"""
python_spec = self.spec['python']
include_dir = python_spec.package.get_python_inc()
include_dir = join_path(
python_spec.prefix, python_spec.package.config_vars['python_inc']['false'])
return [
self.define('Python_EXECUTABLE', str(python_spec.command)),
self.define('Python_INCLUDE_DIR', include_dir)
Expand Down
3 changes: 2 additions & 1 deletion var/spack/repos/builtin/packages/migraphx/package.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ def cmake_python_hints(self):
CMake based on current spec
"""
python_spec = self.spec['python']
include_dir = python_spec.package.get_python_inc()
include_dir = join_path(
python_spec.prefix, python_spec.package.config_vars['python_inc']['false'])
return [
self.define('Python_INCLUDE_DIR', include_dir)
]
Expand Down
176 changes: 73 additions & 103 deletions var/spack/repos/builtin/packages/python/package.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#
# SPDX-License-Identifier: (Apache-2.0 OR MIT)

import json
import os
import platform
import re
Expand Down Expand Up @@ -231,8 +232,8 @@ class Python(AutotoolsPackage):

conflicts('%nvhpc')

# Used to cache home locations, since computing them might be expensive
_homes = {}
# Used to cache various attributes that are expensive to compute
_config_vars = {}

# An in-source build with --enable-optimizations fails for python@3.X
build_directory = 'spack-build'
Expand Down Expand Up @@ -506,7 +507,7 @@ def filter_compilers(self):
kwargs = {'ignore_absent': True, 'backup': False, 'string': True}

filenames = [
self.get_sysconfigdata_name(), self.get_makefile_filename()
self.get_sysconfigdata_name(), self.config_vars['makefile_filename']
]

filter_file(spack_cc, self.compiler.cc, *filenames, **kwargs)
Expand Down Expand Up @@ -686,87 +687,59 @@ def print_string(self, string):
else:
return 'print({0})'.format(string)

def get_config_var(self, key):
"""Return the value of a single variable. Wrapper around
``distutils.sysconfig.get_config_var()``."""

cmd = 'from distutils.sysconfig import get_config_var; '
cmd += self.print_string("get_config_var('{0}')".format(key))

return self.command('-c', cmd, output=str).strip()

def get_config_h_filename(self):
"""Return the full path name of the configuration header.
Wrapper around ``distutils.sysconfig.get_config_h_filename()``."""

cmd = 'from distutils.sysconfig import get_config_h_filename; '
cmd += self.print_string('get_config_h_filename()')

return self.command('-c', cmd, output=str).strip()

def get_makefile_filename(self):
"""Return the full path name of ``Makefile`` used to build Python.
Wrapper around ``distutils.sysconfig.get_makefile_filename()``."""

cmd = 'from distutils.sysconfig import get_makefile_filename; '
cmd += self.print_string('get_makefile_filename()')

return self.command('-c', cmd, output=str).strip()

def get_python_inc(self, plat_specific=False, prefix=None):
"""Return the directory for either the general or platform-dependent C
include files. Wrapper around ``distutils.sysconfig.get_python_inc()``.
Parameters:
plat_specific (bool): if true, the platform-dependent include directory
is returned, else the platform-independent directory is returned
prefix (str): prefix to use instead of ``distutils.sysconfig.PREFIX``
Returns:
str: include files directory
"""
# Wrap strings in quotes
if prefix is not None:
prefix = '"{0}"'.format(prefix)

args = 'plat_specific={0}, prefix={1}'.format(plat_specific, prefix)

cmd = 'from distutils.sysconfig import get_python_inc; '
cmd += self.print_string('get_python_inc({0})'.format(args))

return self.command('-c', cmd, output=str).strip()

def get_python_lib(self, plat_specific=False, standard_lib=False, prefix=None):
"""Return the directory for either the general or platform-dependent
library installation. Wrapper around ``distutils.sysconfig.get_python_lib()``.
@property
def config_vars(self):
"""Return a set of variable definitions associated with a Python installation.
Parameters:
plat_specific (bool): if true, the platform-dependent library directory
is returned, else the platform-independent directory is returned
standard_lib (bool): if true, the directory for the standard library is
returned rather than the directory for the installation of
third-party extensions
prefix (str): prefix to use instead of ``distutils.sysconfig.PREFIX``
Wrapper around various ``distutils.sysconfig`` functions.
Returns:
str: library installation directory
dict: variable definitions
"""
# Wrap strings in quotes
if prefix is not None:
prefix = '"{0}"'.format(prefix)
# TODO: distutils is deprecated in Python 3.10 and will be removed in
# Python 3.12, find a different way to access this information.
# Also, calling the python executable disallows us from cross-compiling,
# so we want to try to avoid that if possible.
cmd = """
import json
from distutils.sysconfig import (
get_config_vars,
get_config_h_filename,
get_makefile_filename,
get_python_inc,
get_python_lib,
)
config = get_config_vars()
config['config_h_filename'] = get_config_h_filename()
config['makefile_filename'] = get_makefile_filename()
config['python_inc'] = {}
config['python_lib'] = {}
for plat_specific in [True, False]:
config['python_inc'][plat_specific] = get_python_inc(plat_specific, prefix='')
config['python_lib'][plat_specific] = {}
for standard_lib in [True, False]:
config['python_lib'][plat_specific][standard_lib] = get_python_lib(
plat_specific, standard_lib, prefix=''
)
%s
""" % self.print_string("json.dumps(config)")

args = 'plat_specific={0}, standard_lib={1}, prefix={2}'.format(
plat_specific, standard_lib, prefix)

cmd = 'from distutils.sysconfig import get_python_lib; '
cmd += self.print_string('get_python_lib({0})'.format(args))

return self.command('-c', cmd, output=str).strip()
dag_hash = self.spec.dag_hash()
if dag_hash not in self._config_vars:
try:
config = json.loads(self.command('-c', cmd, output=str))
except (ProcessError, RuntimeError):
config = {}
self._config_vars[dag_hash] = config
return self._config_vars[dag_hash]

def get_sysconfigdata_name(self):
"""Return the full path name of the sysconfigdata file."""

libdest = self.get_config_var('LIBDEST')
libdest = self.config_vars['LIBDEST']

filename = '_sysconfigdata.py'
if self.spec.satisfies('@3.6:'):
Expand All @@ -790,34 +763,31 @@ def home(self):
determine exactly where it is installed. Fall back on
``spec['python'].prefix`` if that doesn't work."""

dag_hash = self.spec.dag_hash()
if dag_hash not in self._homes:
try:
prefix = self.get_config_var('prefix')
except ProcessError:
prefix = self.prefix
self._homes[dag_hash] = Prefix(prefix)
return self._homes[dag_hash]
if 'prefix' in self.config_vars:
prefix = self.config_vars['prefix']
else:
prefix = self.prefix
return Prefix(prefix)

@property
def libs(self):
# Spack installs libraries into lib, except on openSUSE where it
# installs them into lib64. If the user is using an externally
# installed package, it may be in either lib or lib64, so we need
# to ask Python where its LIBDIR is.
libdir = self.get_config_var('LIBDIR')
libdir = self.config_vars['LIBDIR']

# In Ubuntu 16.04.6 and python 2.7.12 from the system, lib could be
# in LBPL
# https://mail.python.org/pipermail/python-dev/2013-April/125733.html
libpl = self.get_config_var('LIBPL')
libpl = self.config_vars['LIBPL']

# The system Python installation on macOS and Homebrew installations
# install libraries into a Frameworks directory
frameworkprefix = self.get_config_var('PYTHONFRAMEWORKPREFIX')
frameworkprefix = self.config_vars['PYTHONFRAMEWORKPREFIX']

if '+shared' in self.spec:
ldlibrary = self.get_config_var('LDLIBRARY')
ldlibrary = self.config_vars['LDLIBRARY']

if os.path.exists(os.path.join(libdir, ldlibrary)):
return LibraryList(os.path.join(libdir, ldlibrary))
Expand All @@ -829,7 +799,7 @@ def libs(self):
msg = 'Unable to locate {0} libraries in {1}'
raise RuntimeError(msg.format(ldlibrary, libdir))
else:
library = self.get_config_var('LIBRARY')
library = self.config_vars['LIBRARY']

if os.path.exists(os.path.join(libdir, library)):
return LibraryList(os.path.join(libdir, library))
Expand All @@ -841,16 +811,16 @@ def libs(self):

@property
def headers(self):
try:
config_h = self.get_config_h_filename()
if 'config_h_filename' in self.config_vars:
config_h = self.config_vars['config_h_filename']

if not os.path.exists(config_h):
includepy = self.get_config_var('INCLUDEPY')
includepy = self.config_vars['INCLUDEPY']
msg = 'Unable to locate {0} headers in {1}'
raise RuntimeError(msg.format(self.name, includepy))

headers = HeaderList(config_h)
except ProcessError:
else:
headers = find_headers(
'pyconfig', self.prefix.include, recursive=True)
config_h = headers[0]
Expand All @@ -871,9 +841,9 @@ def python_include_dir(self):
Returns:
str: include files directory
"""
try:
return self.get_python_inc(prefix='')
except (ProcessError, RuntimeError):
if 'python_inc' in self.config_vars:
return self.config_vars['python_inc']['false']
else:
return os.path.join('include', 'python{0}'.format(self.version.up_to(2)))

@property
Expand All @@ -895,9 +865,9 @@ def python_lib_dir(self):
Returns:
str: standard library directory
"""
try:
return self.get_python_lib(standard_lib=True, prefix='')
except (ProcessError, RuntimeError):
if 'python_lib' in self.config_vars:
return self.config_vars['python_lib']['false']['true']
else:
return os.path.join('lib', 'python{0}'.format(self.version.up_to(2)))

@property
Expand All @@ -919,9 +889,9 @@ def site_packages_dir(self):
Returns:
str: site-packages directory
"""
try:
return self.get_python_lib(prefix='')
except (ProcessError, RuntimeError):
if 'python_lib' in self.config_vars:
return self.config_vars['python_lib']['false']['false']
else:
return self.default_site_packages_dir

@property
Expand Down Expand Up @@ -978,8 +948,8 @@ def setup_dependent_build_environment(self, env, dependent_spec):
for compile_var, link_var in [('CC', 'LDSHARED'),
('CXX', 'LDCXXSHARED')]:
# First, we get the values from the sysconfigdata:
config_compile = self.get_config_var(compile_var)
config_link = self.get_config_var(link_var)
config_compile = self.config_vars[compile_var]
config_link = self.config_vars[link_var]

# The dependent environment will have the compilation command set to
# the following:
Expand Down

0 comments on commit 6eb942c

Please sign in to comment.