Skip to content
Snippets Groups Projects
  1. Dec 24, 2019
    • Todd Gamblin's avatar
      performance: dont' read `spec.yaml` files twice in view regeneration · e22d3250
      Todd Gamblin authored
      `ViewDescriptor.regenerate()` calls `get_all_specs()`, which reads
      `spec.yaml` files, which is slow.  It's fine to do this once, but
      `view.remove_specs()` *also* calls it immediately afterwards.
      
      - [x] Pass the result of `get_all_specs()` as an optional parameter to
        `view.remove_specs()` to avoid reading `spec.yaml` files twice.
      e22d3250
    • Todd Gamblin's avatar
      performance: don't recompute hashes when regenerating environments · e3939b0c
      Todd Gamblin authored
      `ViewDescriptor.regenerate()` was copying specs and stripping build
      dependencies, which clears `_hash` and other cached fields on concrete
      specs, which causes a bunch of YAML hashes to be recomputed.
      
      - [x] Preserve the `_hash` and `_normal` fields on stripped specs, as
        these will be unchanged.
      e3939b0c
    • Todd Gamblin's avatar
      performance: reduce system calls required for remove_dead_links · f0136873
      Todd Gamblin authored
      `os.path.exists()` will report False if the target of a symlink doesn't
      exist, so we can avoid a costly call to realpath here.
      f0136873
    • Todd Gamblin's avatar
      performance: only regenerate env views once in `spack install` · 79ddf6cf
      Todd Gamblin authored
      `spack install` previously concretized, writes the entire environment
      out, regenerated views, then wrote and regenerated views
      again. Regenerating views is slow, so ensure that we only do that once.
      
      - [x] add an option to env.write() to skip view regeneration
      
      - [x] add a note on whether regenerate_views() shouldn't just be a
        separate operation -- not clear if we want to keep it as part of write
        to ensure consistency, or take it out to avoid performance issues.
      79ddf6cf
    • Todd Gamblin's avatar
      performance: add read transactions for `install_all()` and `install()` · be6d7db2
      Todd Gamblin authored
      Environments need to read the DB a lot when installing all specs.
      
      - [x] Put a read transaction around `install_all()` and `install()`
        to avoid repeated locking
      be6d7db2
    • Todd Gamblin's avatar
      lock transactions: avoid redundant reading in write transactions · d87ededd
      Todd Gamblin authored
      Our `LockTransaction` class was reading overly aggressively.  In cases
      like this:
      
      ```
      1  with spack.store.db.read_transaction():
      2    with spack.store.db.write_transaction():
      3      ...
      ```
      
      The `ReadTransaction` on line 1 would read in the DB, but the
      WriteTransaction on line 2 would read in the DB *again*, even though we
      had a read lock the whole time.  `WriteTransaction`s were only
      considering nested writes to decide when to read, but they didn't know
      when we already had a read lock.
      
      - [x] `Lock.acquire_write()` return `False` in cases where we already had
             a read lock.
      d87ededd
    • Todd Gamblin's avatar
      lock transactions: ensure that nested write transactions write · b3a5f2e3
      Todd Gamblin authored
      If a write transaction was nested inside a read transaction, it would not
      write properly on release, e.g., in a sequence like this, inside our
      `LockTransaction` class:
      
      ```
      1  with spack.store.db.read_transaction():
      2    with spack.store.db.write_transaction():
      3      ...
      4  with spack.store.db.read_transaction():
         ...
      ```
      
      The WriteTransaction on line 2 had no way of knowing that its
      `__exit__()` call was the last *write* in the nesting, and it would skip
      calling its write function.
      
      The `__exit__()` call of the `ReadTransaction` on line 1 wouldn't know
      how to write, and the file would never be written.
      
      The DB would be correct in memory, but the `ReadTransaction` on line 4
      would re-read the whole DB assuming that other processes may have
      modified it.  Since the DB was never written, we got stale data.
      
      - [x] Make `Lock.release_write()` return `True` whenever we release the
            *last write* in a nest.
      b3a5f2e3
    • Todd Gamblin's avatar
      lock transactions: fix non-transactional writes · 98577e3a
      Todd Gamblin authored
      Lock transactions were actually writing *after* the lock was
      released. The code was looking at the result of `release_write()` before
      writing, then writing based on whether the lock was released.  This is
      pretty obviously wrong.
      
      - [x] Refactor `Lock` so that a release function can be passed to the
            `Lock` and called *only* when a lock is really released.
      
      - [x] Refactor `LockTransaction` classes to use the release function
        instead of checking the return value of `release_read()` / `release_write()`
      98577e3a
    • Todd Gamblin's avatar
      performance: avoid repeated DB locking on view generation · a85b9070
      Todd Gamblin authored
      `ViewDescriptor.regenerate()` checks repeatedly whether packages are
      installed and also does a lot of DB queries.  Put a read transaction
      around the whole thing to avoid repeatedly locking and unlocking the DB.
      a85b9070
    • Todd Gamblin's avatar
      performance: speed up `spack find` in environments · 91ea90c2
      Todd Gamblin authored
      `Environment.added_specs()` has a loop around calls to
      `Package.installed()`, which can result in repeated DB queries.  Optimize
      this with a read transaction in `Environment`.
      91ea90c2
    • Todd Gamblin's avatar
      performance: `spack spec` should use a read transacction with -I · 5bdba988
      Todd Gamblin authored
      `spack spec -I` queries the database for installation status and should
      use a read transaction around calls to `Spec.tree()`.
      5bdba988
    • Todd Gamblin's avatar
      concretization: improve performance by avoiding database locks · cbf85534
      Todd Gamblin authored
      Checks for deprecated specs were repeatedly taking out read locks on the
      database, which can be very slow.
      
      - [x] put a read transaction around the deprecation check
      cbf85534
    • Todd Gamblin's avatar
      performance: memoize spack.architecture.get_platform() · 48befd67
      Todd Gamblin authored
      `get_platform()` is pretty expensive and can be called many times in a
      spack invocation.
      
      - [x] memoize `get_platform()`
      48befd67
    • Sajid Ali's avatar
    • Peter Josef Scheibel's avatar
      63915613
    • Peter Josef Scheibel's avatar
      Mirrors: skip attempts to fetch BundlePackages · 587c650b
      Peter Josef Scheibel authored
      BundlePackages use a noop fetch strategy. The mirror logic was assuming
      that the fetcher had a resource to cach after performing a fetch. This adds
      a special check to skip caching if the stage is associated with a
      BundleFetchStrategy. Note that this should allow caching resources
      associated with BundlePackages.
      587c650b
    • Peter Josef Scheibel's avatar
      Mirrors: avoid re-downloading patches · d7142862
      Peter Josef Scheibel authored
      When updating a mirror, Spack was re-retrieving all patches (since the
      fetch logic for patches is separate). This updates the patch logic to
      allow the mirror logic to avoid this.
      d7142862
    • Peter Josef Scheibel's avatar
      Mirrors: perform checksum of fetched sources · a69b3c85
      Peter Josef Scheibel authored
      Since cache_mirror does the fetch itself, it also needs to do the
      checksum itself if it wants to verify that the source stored in the
      mirror is valid. Note that this isn't strictly required because fetching
      (including from mirrors) always separately verifies the checksum.
      a69b3c85
    • Peter Josef Scheibel's avatar
      Mirrors: fix cosmetic symlink targets · 98b498c6
      Peter Josef Scheibel authored
      The targets for the cosmetic paths in mirrrors were being calculated
      incorrectly as of fb3a3ba9: the symlinks used relative paths as targets,
      and the relative path was computed relative to the wrong directory.
      98b498c6
    • Peter Josef Scheibel's avatar
      Allow repeated invocations of 'mirror create' · 64209dda
      Peter Josef Scheibel authored
      When creating a cosmetic symlink for a resource in a mirror, remove
      it if it already exists. The symlink is removed in case the logic to
      create the symlink has changed.
      64209dda
    • Paul Ferrell's avatar
      mirror bug fixes: symlinks, duplicate patch names, and exception handling (#13789) · c15e55c6
      Paul Ferrell authored
      * Some packages (e.g. mpfr at the time of this patch) can have patches
        with the same name but different contents (which apply to different
        versions of the package). This appends part of the patch hash to the
        cache file name to avoid conflicts.
      * Some exceptions which occur during fetching are not a subclass of
        SpackError and therefore do not have a 'message' attribute. This
        updates the logic for mirroring a single spec (add_single_spec)
        to produce an appropriate error message in that case (where before
        it failed with an AttributeError)
      * In various circumstances, a mirror can contain the universal storage
        path but not a cosmetic symlink; in this case it would not generate
        a symlink. Now "spack mirror create" will create a symlink for any
        package that doesn't have one.
      c15e55c6
  2. Dec 05, 2019
    • Todd Gamblin's avatar
    • Todd Gamblin's avatar
      version bump: 0.13.2 · 46b68263
      Todd Gamblin authored
      46b68263
    • Peter Scheibel's avatar
      Bugfix: allow missing modules if they are blacklisted (#13540) · 03a5771b
      Peter Scheibel authored
      `spack module loads` and `spack module find` previously failed if any upstream modules were missing.  This prevented it from being used with upstreams (or, really, any spack instance) that blacklisted modules.
      
      This PR makes module finding is now more lenient (especially for blacklisted modules).
      
      - `spack module find` now does not report an error if the spec is blacklisted
        - instead, it prints a single warning if any modules will be omitted from the loads file
        - It comments the missing modules out of the loads file so the user can see what's missing
        - Debug messages are also printed so users can check this with `spack -d...`
      
      - also added tests for new functionality
      03a5771b
  3. Dec 02, 2019
  4. Nov 17, 2019
  5. Nov 15, 2019
  6. Nov 14, 2019
  7. Nov 05, 2019
  8. Nov 04, 2019
    • Omar Padron's avatar
      bugfix: fetch prefers to fetch local mirrors over remote resources (#13545) · edf95483
      Omar Padron authored
      - [x] insert at beginning of list so fetch grabs local mirrors before remote resources
      - [x] update the S3FetchStrategy so that it throws a SpackError if the fetch fails.  
            Before, it was throwing URLError, which was not being caught in stage.py.
      - [x] move error handling out of S3FetchStrategy and into web_util.read_from_url()
      - [x] pass string instead of URLError to SpackWebError
      edf95483
Loading