Merge pull request #2335 from retronym/topic/opt-file-access

Optimize file metadata access

5 files changed, 86 insertions, 12 deletions

diff --git a/src/compiler/scala/tools/nsc/util/ClassPath.scala b/src/compiler/scala/tools/nsc/util/ClassPath.scala
index aa4128f1a7..7f9b81e1ec 100644
--- a/src/compiler/scala/tools/nsc/util/ClassPath.scala
+++ b/src/compiler/scala/tools/nsc/util/ClassPath.scala
@@ -281,11 +281,24 @@ class DirectoryClassPath(val dir: AbstractFile, val context: ClassPathContext[Ab
   private def traverse() = {
     val classBuf   = immutable.Vector.newBuilder[ClassRep]
     val packageBuf = immutable.Vector.newBuilder[DirectoryClassPath]
-    dir foreach { f =>
-      if (!f.isDirectory && validClassFile(f.name))
-        classBuf += ClassRep(Some(f), None)
-      else if (f.isDirectory && validPackage(f.name))
-        packageBuf += new DirectoryClassPath(f, context)
+    dir foreach {
+      f =>
+        // Optimization: We assume the file was not changed since `dir` called
+        // `Path.apply` and categorized existent files as `Directory`
+        // or `File`.
+        val isDirectory = f match {
+          case pf: io.PlainFile => pf.givenPath match {
+            case _: io.Directory => true
+            case _: io.File      => false
+            case _               => f.isDirectory
+          }
+          case _ =>
+            f.isDirectory
+        }
+        if (!isDirectory && validClassFile(f.name))
+          classBuf += ClassRep(Some(f), None)
+        else if (isDirectory && validPackage(f.name))
+          packageBuf += new DirectoryClassPath(f, context)
     }
     (packageBuf.result(), classBuf.result())
   }
diff --git a/src/reflect/scala/reflect/io/AbstractFile.scala b/src/reflect/scala/reflect/io/AbstractFile.scala
index 8b69efc749..4ac56da628 100644
--- a/src/reflect/scala/reflect/io/AbstractFile.scala
+++ b/src/reflect/scala/reflect/io/AbstractFile.scala
@@ -11,6 +11,7 @@ import java.io.{ FileOutputStream, IOException, InputStream, OutputStream, Buffe
 import java.io.{ File => JFile }
 import java.net.URL
 import scala.collection.mutable.ArrayBuffer
+import scala.reflect.internal.util.Statistics
 
 /**
  * An abstraction over files for use in the reflection/compiler libraries.
@@ -112,7 +113,10 @@ abstract class AbstractFile extends Iterable[AbstractFile] {
   def underlyingSource: Option[AbstractFile] = None
 
   /** Does this abstract file denote an existing file? */
-  def exists: Boolean = (file eq null) || file.exists
+  def exists: Boolean = {
+    if (Statistics.canEnable) Statistics.incCounter(IOStats.fileExistsCount)
+    (file eq null) || file.exists
+  }
 
   /** Does this abstract file represent something which can contain classfiles? */
   def isClassContainer = isDirectory || (file != null && (extension == "jar" || extension == "zip"))
diff --git a/src/reflect/scala/reflect/io/IOStats.scala b/src/reflect/scala/reflect/io/IOStats.scala
new file mode 100644
index 0000000000..64e1e952cd
--- /dev/null
+++ b/src/reflect/scala/reflect/io/IOStats.scala
@@ -0,0 +1,31 @@
+package scala.reflect.io
+
+import scala.reflect.internal.util.Statistics
+
+// Due to limitations in the Statistics machinery, these are only
+// reported if this patch is applied.
+//
+// --- a/src/reflect/scala/reflect/internal/util/Statistics.scala
+// +++ b/src/reflect/scala/reflect/internal/util/Statistics.scala
+// @@ -109,7 +109,7 @@ quant)
+//     *  Quantities with non-empty prefix are printed in the statistics info.
+//     */
+//    trait Quantity {
+// -    if (enabled && prefix.nonEmpty) {
+// +    if (prefix.nonEmpty) {
+//        val key = s"${if (underlying != this) underlying.prefix else ""}/$prefix"
+//        qs(key) = this
+//      }
+// @@ -243,7 +243,7 @@ quant)
+//     *
+//     *  to remove all Statistics code from build
+//     */
+// -  final val canEnable = _enabled
+// +  final val canEnable = true // _enabled
+//
+// We can commit this change as the first diff reverts a fix for an IDE memory leak.
+private[io] object IOStats {
+  val fileExistsCount      = Statistics.newCounter("# File.exists calls")
+  val fileIsDirectoryCount = Statistics.newCounter("# File.isDirectory calls")
+  val fileIsFileCount      = Statistics.newCounter("# File.isFile calls")
+}
diff --git a/src/reflect/scala/reflect/io/Path.scala b/src/reflect/scala/reflect/io/Path.scala
index 44fb41a1cd..56d4faed99 100644
--- a/src/reflect/scala/reflect/io/Path.scala
+++ b/src/reflect/scala/reflect/io/Path.scala
@@ -13,6 +13,7 @@ import java.io.{ File => JFile }
 import java.net.{ URI, URL }
 import scala.util.Random.alphanumeric
 import scala.language.implicitConversions
+import scala.reflect.internal.util.Statistics
 
 /** An abstraction for filesystem paths.  The differences between
  *  Path, File, and Directory are primarily to communicate intent.
@@ -57,8 +58,18 @@ object Path {
 
   def apply(path: String): Path = apply(new JFile(path))
   def apply(jfile: JFile): Path = try {
-    if (jfile.isFile) new File(jfile)
-    else if (jfile.isDirectory) new Directory(jfile)
+    def isFile = {
+      if (Statistics.canEnable) Statistics.incCounter(IOStats.fileIsFileCount)
+      jfile.isFile
+    }
+
+    def isDirectory = {
+      if (Statistics.canEnable) Statistics.incCounter(IOStats.fileIsDirectoryCount)
+      jfile.isDirectory
+    }
+
+    if (isFile) new File(jfile)
+    else if (isDirectory) new Directory(jfile)
     else new Path(jfile)
   } catch { case ex: SecurityException => new Path(jfile) }
 
@@ -187,10 +198,19 @@ class Path private[io] (val jfile: JFile) {
   // Boolean tests
   def canRead = jfile.canRead()
   def canWrite = jfile.canWrite()
-  def exists = try jfile.exists() catch { case ex: SecurityException => false }
+  def exists = {
+    if (Statistics.canEnable) Statistics.incCounter(IOStats.fileExistsCount)
+    try jfile.exists() catch { case ex: SecurityException => false }
+  }
 
-  def isFile = try jfile.isFile() catch { case ex: SecurityException => false }
-  def isDirectory = try jfile.isDirectory() catch { case ex: SecurityException => false }
+  def isFile = {
+    if (Statistics.canEnable) Statistics.incCounter(IOStats.fileIsFileCount)
+    try jfile.isFile() catch { case ex: SecurityException => false }
+  }
+  def isDirectory = {
+    if (Statistics.canEnable) Statistics.incCounter(IOStats.fileIsDirectoryCount)
+    try jfile.isDirectory() catch { case ex: SecurityException => false }
+  }
   def isAbsolute = jfile.isAbsolute()
   def isEmpty = path.length == 0
 
diff --git a/src/reflect/scala/reflect/io/PlainFile.scala b/src/reflect/scala/reflect/io/PlainFile.scala
index 31df78f995..b892fe7cef 100644
--- a/src/reflect/scala/reflect/io/PlainFile.scala
+++ b/src/reflect/scala/reflect/io/PlainFile.scala
@@ -56,8 +56,14 @@ class PlainFile(val givenPath: Path) extends AbstractFile {
 
   /** Returns all abstract subfiles of this abstract directory. */
   def iterator: Iterator[AbstractFile] = {
+    // Optimization: Assume that the file was not deleted and did not have permissions changed
+    // between the call to `list` and the iteration. This saves a call to `exists`.
+    def existsFast(path: Path) = path match {
+      case (_: Directory | _: io.File) => true
+      case _                           => path.exists
+    }
     if (!isDirectory) Iterator.empty
-    else givenPath.toDirectory.list filter (_.exists) map (new PlainFile(_))
+    else givenPath.toDirectory.list filter existsFast map (new PlainFile(_))
   }
 
   /**