performance: only regenerate env views once in `spack install`

`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.

2 files changed, 29 insertions, 9 deletions

diff --git a/lib/spack/spack/cmd/install.py b/lib/spack/spack/cmd/install.py
index 8fd63beede..ab012eaead 100644
--- a/lib/spack/spack/cmd/install.py
+++ b/lib/spack/spack/cmd/install.py
@@ -229,9 +229,14 @@ def install(parser, args, **kwargs):
             if not args.only_concrete:
                 concretized_specs = env.concretize()
                 ev.display_specs(concretized_specs)
-                env.write()
+
+                # save view regeneration for later, so that we only do it
+                # once, as it can be slow.
+                env.write(regenerate_views=False)
+
             tty.msg("Installing environment %s" % env.name)
             env.install_all(args)
+            env.regenerate_views()
             return
         else:
             tty.die("install requires a package argument or a spack.yaml file")
diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py
index 2a2be00425..5b526bbe11 100644
--- a/lib/spack/spack/environment.py
+++ b/lib/spack/spack/environment.py
@@ -1179,7 +1179,12 @@ class Environment(object):
         self.specs_by_hash[h] = concrete
 
     def install_all(self, args=None):
-        """Install all concretized specs in an environment."""
+        """Install all concretized specs in an environment.
+
+        Note: this does not regenerate the views for the environment;
+        that needs to be done separately with a call to write().
+
+        """
         with spack.store.db.read_transaction():
             for concretized_hash in self.concretized_order:
                 spec = self.specs_by_hash[concretized_hash]
@@ -1200,8 +1205,6 @@ class Environment(object):
                         os.remove(build_log_link)
                     os.symlink(spec.package.build_log_path, build_log_link)
 
-            self.regenerate_views()
-
     def all_specs_by_hash(self):
         """Map of hashes to spec for all specs in this environment."""
         # Note this uses dag-hashes calculated without build deps as keys,
@@ -1367,10 +1370,17 @@ class Environment(object):
             self.concretized_order = [
                 old_hash_to_new.get(h, h) for h in self.concretized_order]
 
-    def write(self):
+    def write(self, regenerate_views=True):
         """Writes an in-memory environment to its location on disk.
 
-        This will also write out package files for each newly concretized spec.
+        Write out package files for each newly concretized spec.  Also
+        regenerate any views associated with the environment, if
+        regenerate_views is True.
+
+        Arguments:
+            regenerate_views (bool): regenerate views as well as
+                writing if True.
+
         """
         # ensure path in var/spack/environments
         fs.mkdirp(self.path)
@@ -1478,9 +1488,14 @@ class Environment(object):
             with fs.write_tmp_and_move(self.manifest_path) as f:
                 _write_yaml(self.yaml, f)
 
-        # TODO: for operations that just add to the env (install etc.) this
-        # could just call update_view
-        self.regenerate_views()
+        # TODO: rethink where this needs to happen along with
+        # writing. For some of the commands (like install, which write
+        # concrete specs AND regen) this might as well be a separate
+        # call.  But, having it here makes the views consistent witht the
+        # concretized environment for most operations.  Which is the
+        # special case?
+        if regenerate_views:
+            self.regenerate_views()
 
     def __enter__(self):
         self._previous_active = _active_environment