Faster virtuals and concretization #1015

tgamblin · 2016-06-02T08:31:31Z

This should fix #676.

@citibeth, @davydden, @alalazo, @becker33, @mplegendre and others will be interested in this. The changes here bring deal concretization down to around 3 seconds from around 20. dealii might be one of the most complicated packages in off-the-shelf spack. We have larger ones internally and so does @citibeth, so hopefully this will ease their life a bit.

Things in this PR (or related to it):

e8b4d5f (already in develop because it was a major time saver) fixes large amount of time spent in re.Scanner. A Lexer (implemented with re.Scanner), which compiles regexes, was being constructed every time we parse a spec, and that is expensive. Since spec parsing is a large part of evaluating directives like depends_on, this caused package.py import to take a long time. Adding a single lexer for all parsers fixes this.
Cache the ProviderIndex. We have to import all package.py files when we concretize specs that have virtual dependencies, because we have to find providers but we don't know which package.py files actually call provides. The ProviderIndex holds that information and is now cached per package repository and automatically updated when package.py files are newer than the index. It can also be updated incrementally, so updates after the first will not be very expensive.
Lazy evaluation of all_package_names() so that processing small specs doesn't even look at most package.py files.
Faster hash key function for compiler flags (FlagMap) -- uses tuples and doesn't manipulate strings. This saves a second or so for dealii.
To make the ProviderIndex compact, Spec.to_yaml won't write out empty fields in abstract specs. This should not affect things like spec.yaml or the install database, as those only contain concrete specs.
Allow ProviderIndex caches to be stored in ~/.spack/caches when $repo/index.yaml is not writable. This would let users cache providers even for multi-user spack repos.
Tests for the new ProviderIndex cache.

All of these commits attack the problem that concretization (and running many spack commands) require some fairly expensive operations to happen for every package.py file. This PR reduces the cost of these operations and avoids O(num packages) operations where it can. In the best case, only has to stat each package.py file instead of loading all of them and parsing specs. Generally just runningstat is pretty fast.

The concretization algorithm is still at least quadratic, but the DAGs are still small by modern standards in Spack, so this is not killing us yet.

citibeth · 2016-06-02T14:02:34Z

Is this ready to beta test in regular use?
On Jun 2, 2016 4:54 AM, "Todd Gamblin" [email protected] wrote:

@citibeth https://github.com/citibeth, @davydden
https://github.com/davydden, @alalazo https://github.com/alalazo,
@becker33 https://github.com/becker33, @mplegendre
https://github.com/mplegendre and others will be interested in this.
The changes here bring deal concretization down to around 3 seconds from
around 20. dealii might be one of the most complicated packages in
off-the-shelf spack. We have larger ones internally and so does @citibeth
https://github.com/citibeth, so hopefully this will ease their life a
bit.

Things in this PR (or related to it):

e8b4d5f
https://github.com/LLNL/spack/commit/e8b4d5fb6ffe18bcb93a86d00fbfb1ed37d0db3b
(already in develop) fixes large amount of time spent in re.Scanner. A
Lexer (implemented with re.Scanner), which compiles regexes, was being
constructed every time we parse a spec, and that is expensive. Since spec
parsing is a large part of evaluating directives like depends_on, this
caused package.py import to take a long time. Adding a single lexer
for all parsers fixes this.

Cache the ProviderIndex. We have to import all package.py files
when we concretize specs that have virtual dependencies, because we have to
find providers but we don't know which package.py files actually call
provides. The ProviderIndex holds that information and is now cached
per package repository and automatically updated when package.py files
are newer than the index. It can also be updated incrementally, so updates
after the first will not be very expensive.

Lazy evaluation of all_package_names() so that processing small
specs doesn't even look at most package.py files.

Faster hash key function for compiler flags (FlagMap) -- uses tuples
and doesn't manipulate strings. This saves a second or so for dealii.

To make the ProviderIndex compact, Spec.to_yaml won't write out
empty fields in abstract specs. This should not affect things like
spec.yaml or the install database, as those only contain concrete
specs.

[] Tests for the new ProviderIndex cache.

[] Allow ProviderIndex caches to be stored in ~/.spack/caches when
$repo/index.yaml is not writable. This would let users cache providers
even for multi-user spack repos.

I don't believe it's the actual algorithm that's hurting concretization

at the moment

You can view, comment on, or merge this pull request online at:

https://github.com/LLNL/spack/pull/1015
Commit Summary

Lazily evaluate all_package_names in repository.py

More compact YAML formatting for abstract specs.

Make ProviderIndex yaml-izable.

Make ProviderIndexes mergeable, so we can cache them per-repo.

Add a ProviderIndex cache.

Remove vestigial methods from Package.

Faster key in FlagMap._cmp_key

Fix namespace support in Repo.get_pkg_class()

File Changes

M .gitignore https://github.com/LLNL/spack/pull/1015/files#diff-0
(2)

M lib/spack/spack/database.py (7)

M lib/spack/spack/package.py (37)

M lib/spack/spack/repository.py (199)

M lib/spack/spack/spec.py (65)

M lib/spack/spack/test/init.py
https://github.com/LLNL/spack/pull/1015/files#diff-5 (2)

M lib/spack/spack/test/spec_yaml.py (3)

A lib/spack/spack/test/virtual.py (49)

M lib/spack/spack/virtual.py (97)

Patch Links:

https://github.com/LLNL/spack/pull/1015.patch

https://github.com/LLNL/spack/pull/1015.diff

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/LLNL/spack/pull/1015, or mute the thread
https://github.com/notifications/unsubscribe/AB1cdwk5SuLRA0RKdaohlTbO8WaYdjoSks5qHpougaJpZM4IsUcF
.

tgamblin · 2016-06-02T16:05:28Z

@citibeth I think so. The only reason I have not merged it yet is that I have not written unit tests for the ProviderIndex cache, and I want to ensure the cache works even if you can't read the repository directory.

If you want to beta test it, the only thing you would need to remember is that if things go wrong, remove $repo_root/index.yaml and $repo_root/lock. If you are just using the builtin spack repository, $repo_root is $spack_root/var/spack/repos/builtin.

willelm · 2016-06-02T20:52:45Z

I've been frustrated by speed and eager to try the fix. I merged with 9411cb1, and I think there's an issue with python activation. Try building py-ipython and activating it. It builds fine but gives errors when I start it about readline services being unavailable. Readline is clearly a dependency of python in the spec, but doesn't seem to be followed.

citibeth · 2016-06-21T23:07:01Z

I got the problem below. Maybe my merge was bad.

[rpfische@gs611-gibbs spack]$ spack install patchelf
Traceback (most recent call last):
  File "/home2/rpfische/spack/bin/spack", line 179, in <module>
    main()
  File "/home2/rpfische/spack/bin/spack", line 157, in main
    return_val = command(parser, args)
  File "/home2/rpfische/spack/lib/spack/spack/cmd/install.py", line 82, in install
    explicit=True)
  File "/home2/rpfische/spack/lib/spack/spack/package.py", line 870, in do_install
    if 'install' in install_phases and spack.install_layout.check_installed(self.spec):
  File "/home2/rpfische/spack/lib/spack/spack/directory_layout.py", line 272, in check_installed
    path = self.path_for_spec(spec)
  File "/home2/rpfische/spack/lib/spack/spack/directory_layout.py", line 138, in path_for_spec
    path = self.relative_path_for_spec(spec)
  File "/home2/rpfische/spack/lib/spack/spack/directory_layout.py", line 208, in relative_path_for_spec
    spec.dag_hash(self.hash_len))
  File "/home2/rpfische/spack/lib/spack/spack/spec.py", line 743, in dag_hash
    self.to_node_dict(), default_flow_style=True, width=sys.maxint)
  File "/home2/rpfische/spack/lib/spack/external/yaml/__init__.py", line 202, in dump
    return dump_all([data], stream, Dumper=Dumper, **kwds)
  File "/home2/rpfische/spack/lib/spack/external/yaml/__init__.py", line 190, in dump_all
    dumper.represent(data)
  File "/home2/rpfische/spack/lib/spack/external/yaml/representer.py", line 28, in represent
    node = self.represent_data(data)
  File "/home2/rpfische/spack/lib/spack/external/yaml/representer.py", line 57, in represent_data
    node = self.yaml_representers[data_types[0]](self, data)
  File "/home2/rpfische/spack/lib/spack/external/yaml/representer.py", line 223, in represent_dict
    return self.represent_mapping(u'tag:yaml.org,2002:map', data)
  File "/home2/rpfische/spack/lib/spack/external/yaml/representer.py", line 123, in represent_mapping
    node_value = self.represent_data(item_value)
  File "/home2/rpfische/spack/lib/spack/external/yaml/representer.py", line 57, in represent_data
    node = self.yaml_representers[data_types[0]](self, data)
  File "/home2/rpfische/spack/lib/spack/external/yaml/representer.py", line 223, in represent_dict
    return self.represent_mapping(u'tag:yaml.org,2002:map', data)
  File "/home2/rpfische/spack/lib/spack/external/yaml/representer.py", line 123, in represent_mapping
    node_value = self.represent_data(item_value)
  File "/home2/rpfische/spack/lib/spack/external/yaml/representer.py", line 41, in represent_data
    if self.ignore_aliases(data):
  File "/home2/rpfische/spack/lib/spack/external/yaml/representer.py", line 142, in ignore_aliases
    if data in [None, ()]:
  File "/home2/rpfische/spack/lib/spack/llnl/util/lang.py", line 240, in <lambda>
    setter('__eq__', lambda s,o: (s is o) or (o is not None and s._cmp_key() == o._cmp_key()))
AttributeError: 'tuple' object has no attribute '_cmp_key'

citibeth · 2016-06-22T21:16:13Z

I've had success with a different merge. This PR has made my life with Spack so much better. I recommend it be merged without delay.

- Don't need to list all packages unless we have to. - Only use the list of all packages for existence checks if we have generated it for some other purpose.

- Don't add empty/absent fields to Spec YAML when they're not there.

- allow a provider index to be stored and re-read.

- Spack will check if the index needs updating, and will only parse all package files if it does. - Spack tries to parse as few package files as necessary.

- modules weren't set properly as attributes in parent modules

- global compiler cache breaks tests that come after this one.

- Make namespace, arch, and dependnecies show up in spec yaml only if they're set. - Lost some of this functionality with deptypes

Major stuff: - Created a FileCache for managing user cache files in Spack. Currently just handles virtuals. - Moved virtual cache from the repository to the home directory so that users do not need write access to Spack repositories to use them. - Refactored `Transaction` class in `database.py` -- moved it to `LockTransaction` in `lock.py` and made it reusable by other classes. Other additions: - Added tests for file cache and transactions. - Added a few more tests for database - Fixed bug in DB where writes could happen even if exceptions were raised during a transaction. - `spack uninstall` now attempts to repair the database when it discovers that a prefix doesn't exist but a DB record does.

tgamblin · 2016-08-09T08:45:12Z

This is ready to merge. I've cleaned up a number of changes, and caching is now done in ~/.spack so it doesn't require write access to Spack repos to work. I also simplified file locking a bit.

It's rebased on current develop, so the merge should be clean. Can @citibeth, @alalazo, @gartung, @davydden, @glennpj try this out and let me know if it helps?

@glennpj: I think your issue in #676 may be something different, but this is worth a try.

davydden · 2016-08-09T18:57:12Z

@tgamblin thanks! sorry, did not have time to check it out, but ought to be fine.

Co-authored-by: Blanco Alonso Jorge <[email protected]>

tgamblin changed the title ~~[WIP] Features/faster virtuals~~ [WIP] Features/faster virtuals and concretization Jun 2, 2016

glennpj mentioned this pull request Aug 3, 2016

Spack is Great, but too slow #676

Closed

tgamblin added 17 commits August 8, 2016 21:04

Lazily evaluate all_package_names in repository.py

7aaad89

- Don't need to list all packages unless we have to. - Only use the list of all packages for existence checks if we have generated it for some other purpose.

More compact YAML formatting for abstract specs.

025609c

- Don't add empty/absent fields to Spec YAML when they're not there.

Make ProviderIndex yaml-izable.

bf02899

- allow a provider index to be stored and re-read.

Make ProviderIndexes mergeable, so we can cache them per-repo.

cf2f902

Add a ProviderIndex cache.

faa0a0e

- Spack will check if the index needs updating, and will only parse all package files if it does. - Spack tries to parse as few package files as necessary.

Remove vestigial methods from Package.

37fc258

Faster key in FlagMap._cmp_key

ab049ec

Fix namespace support in Repo.get_pkg_class()

1f5a21d

rename virtual module to provider_index

ce6ac93

fix scoping issue.

4de45c2

Fix iterator invalidation issues.

5e50243

WIP

d195576

Fix issues with import order in tests.

e5743db

- modules weren't set properly as attributes in parent modules

Make compiler command test last until caching is fixed.

5d690c9

- global compiler cache breaks tests that come after this one.

Restore text output in verbose mode.

1339714

Fix bugs with sparse spec printing.

2042e9a

- Make namespace, arch, and dependnecies show up in spec yaml only if they're set. - Lost some of this functionality with deptypes

tgamblin force-pushed the features/faster-virtuals branch from 9411cb1 to 05f222a Compare August 9, 2016 08:41

tgamblin added 2 commits August 9, 2016 02:25

Flake8 fixes

0c75c13

Properly re-raise exceptions from lock context handler.

9d4a36a

tgamblin force-pushed the features/faster-virtuals branch from 95bb387 to 9d4a36a Compare August 9, 2016 09:25

tgamblin changed the title ~~[WIP] Features/faster virtuals and concretization~~ Features/faster virtuals and concretization Aug 9, 2016

tgamblin changed the title ~~Features/faster virtuals and concretization~~ Faster virtuals and concretization Aug 9, 2016

tgamblin merged commit a095fd5 into develop Aug 9, 2016

adamjstewart mentioned this pull request Aug 10, 2016

Fix superclass constructor args. #1495

Merged

tgamblin mentioned this pull request Aug 26, 2016

[WIP] Feature/remote repo #965

Closed

tgamblin deleted the features/faster-virtuals branch October 11, 2016 20:02

tgamblin added concretization performance virtual-dependencies labels Oct 12, 2016

tgamblin mentioned this pull request Oct 12, 2016

Use cYAML if it is available #2010

Merged

tgamblin mentioned this pull request Oct 25, 2016

Problems with Success: 1000 Packages! #2094

Closed

olupton pushed a commit to olupton/spack that referenced this pull request Feb 7, 2022

libsonata-report: Add support for synapse reports (spack#1015)

4297756

Co-authored-by: Blanco Alonso Jorge <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Faster virtuals and concretization #1015

Faster virtuals and concretization #1015

tgamblin commented Jun 2, 2016 •

edited

Loading

citibeth commented Jun 2, 2016

at the moment

tgamblin commented Jun 2, 2016 •

edited

Loading

willelm commented Jun 2, 2016

citibeth commented Jun 21, 2016

citibeth commented Jun 22, 2016

tgamblin commented Aug 9, 2016

davydden commented Aug 9, 2016

Faster virtuals and concretization #1015

Faster virtuals and concretization #1015

Conversation

tgamblin commented Jun 2, 2016 • edited Loading

citibeth commented Jun 2, 2016

at the moment

tgamblin commented Jun 2, 2016 • edited Loading

willelm commented Jun 2, 2016

citibeth commented Jun 21, 2016

citibeth commented Jun 22, 2016

tgamblin commented Aug 9, 2016

davydden commented Aug 9, 2016

tgamblin commented Jun 2, 2016 •

edited

Loading

tgamblin commented Jun 2, 2016 •

edited

Loading