Skip to content
Snippets Groups Projects
  1. Dec 24, 2019
    • Todd Gamblin's avatar
      performance: dont' read `spec.yaml` files twice in view regeneration · d7f2a328
      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.
      d7f2a328
    • Todd Gamblin's avatar
      performance: don't recompute hashes when regenerating environments · 78b84e4a
      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.
      78b84e4a
    • Todd Gamblin's avatar
      performance: reduce system calls required for remove_dead_links · 9b90d7e8
      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.
      9b90d7e8
    • Todd Gamblin's avatar
      performance: only regenerate env views once in `spack install` · c83e365c
      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.
      c83e365c
    • Todd Gamblin's avatar
      performance: add read transactions for `install_all()` and `install()` · 0fb32800
      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
      0fb32800
    • Todd Gamblin's avatar
      lock transactions: avoid redundant reading in write transactions · 6c9467e8
      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.
      6c9467e8
    • Todd Gamblin's avatar
      lock transactions: ensure that nested write transactions write · bb517fdb
      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.
      bb517fdb
    • Todd Gamblin's avatar
      lock transactions: fix non-transactional writes · eb8fc4f3
      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()`
      eb8fc4f3
    • Todd Gamblin's avatar
      performance: avoid repeated DB locking on view generation · 779ac9fe
      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.
      779ac9fe
  2. Dec 23, 2019
  3. Dec 22, 2019
  4. Dec 21, 2019
  5. Dec 20, 2019
Loading