From 76504bac5d696e126d0a06df506d4923fc7962e2 Mon Sep 17 00:00:00 2001 From: Joseph Boyd Date: Fri, 31 Jul 2020 17:49:44 +0000 Subject: [PATCH] scrooge-generator: cache getCanonicalPath value in DirImporter Problem Using a large number of `DirImporter` instances within a `MultiImporter` is very slow. This is because `DirImporter` uses `java.io.File.getCanonicalPath` as part of it's `equals()` and `hashCode()` methods, which are called frequently from `MultiImporter`, esp. in `MultiImporter.deduped()`. `File.getCanonicalPath` makes system IO calls, and is relatively slow because of it. Solution Updates `DirImporter` to cache the result of `getCanonicalPath` so it's only called once. JIRA Issues: AIPIPE-7239 Differential Revision: https://phabricator.twitter.biz/D523368 --- .../com/twitter/scrooge/frontend/Importer.scala | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/scrooge-generator/src/main/scala/com/twitter/scrooge/frontend/Importer.scala b/scrooge-generator/src/main/scala/com/twitter/scrooge/frontend/Importer.scala index 5abd3d53e..a08865b99 100644 --- a/scrooge-generator/src/main/scala/com/twitter/scrooge/frontend/Importer.scala +++ b/scrooge-generator/src/main/scala/com/twitter/scrooge/frontend/Importer.scala @@ -78,7 +78,9 @@ object Importer { } final case class DirImporter(dir: File) extends Importer { - private[scrooge] def canonicalPaths = Seq(dir.getCanonicalPath) // for test + private lazy val dirCanonicalPath = dir.getCanonicalPath + + private[scrooge] def canonicalPaths = Seq(dirCanonicalPath) // for test private[this] def resolve(filename: String): Option[(File, Importer)] = { val file = { @@ -91,7 +93,7 @@ final case class DirImporter(dir: File) extends Importer { if (file.canRead) { // if the file does not exactly locate in dir, we need to add a new relative path val importer = - if (file.getParentFile.getCanonicalPath == dir.getCanonicalPath) + if (file.getParentFile.getCanonicalPath == dirCanonicalPath) this else Importer(file.getParentFile) +: this @@ -117,19 +119,19 @@ final case class DirImporter(dir: File) extends Importer { resolve(filename).map { case (file, _) => file.getCanonicalPath } } - override def toString: String = s"DirImporter(${dir.getCanonicalPath})" + override def toString: String = s"DirImporter($dirCanonicalPath)" // We override equals and hashCode because // the default equality for File does not have the semantics we want. override def equals(other: Any): Boolean = { other match { case that: DirImporter => - this.dir.getCanonicalPath == that.dir.getCanonicalPath + this.dirCanonicalPath == that.dirCanonicalPath case _ => false } } - override def hashCode: Int = this.dir.getCanonicalPath.hashCode + override def hashCode: Int = this.dirCanonicalPath.hashCode } // jar files are just zip files, so this will work with both .jar and .zip files