diff options
author | James Iry <jamesiry@gmail.com> | 2013-04-10 11:39:12 -0700 |
---|---|---|
committer | James Iry <jamesiry@gmail.com> | 2013-04-10 11:39:12 -0700 |
commit | f6323d866f3b9a6fa9a5d0218a47922115974781 (patch) | |
tree | b44706c87599c355459e247f074030fe59af23d6 | |
parent | 53b0180b4659790b52777563ae4b8c3a3a60ce75 (diff) | |
parent | 7072acb786663d554e28a4c355ee4d94478aef54 (diff) | |
download | scala-f6323d866f3b9a6fa9a5d0218a47922115974781.tar.gz scala-f6323d866f3b9a6fa9a5d0218a47922115974781.tar.bz2 scala-f6323d866f3b9a6fa9a5d0218a47922115974781.zip |
Merge pull request #2335 from retronym/topic/opt-file-access
Optimize file metadata access
-rw-r--r-- | src/compiler/scala/tools/nsc/util/ClassPath.scala | 23 | ||||
-rw-r--r-- | src/reflect/scala/reflect/io/AbstractFile.scala | 6 | ||||
-rw-r--r-- | src/reflect/scala/reflect/io/IOStats.scala | 31 | ||||
-rw-r--r-- | src/reflect/scala/reflect/io/Path.scala | 30 | ||||
-rw-r--r-- | src/reflect/scala/reflect/io/PlainFile.scala | 8 |
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(_)) } /** |