From 54e67889f10b39323fb729808b8cd22b4c832910 Mon Sep 17 00:00:00 2001 From: chick Date: Thu, 1 Aug 2019 09:21:52 -0700 Subject: Followup to PR #1142 Fixes a threading bug in where lazy reading of file caused a problem for multithreaded access to the that was read. Changes all uses of io.Source to use new API getText getLines getTextResource getLinesResouce Make style to only import FileUtils and not its methods So code is more explicit as e.g. FileUtils.getText() --- src/main/scala/firrtl/Driver.scala | 4 ++-- src/main/scala/firrtl/FileUtils.scala | 1 + src/main/scala/firrtl/options/phases/GetIncludes.scala | 3 ++- src/main/scala/firrtl/passes/memlib/YamlUtils.scala | 3 ++- src/main/scala/firrtl/stage/phases/DriverCompatibility.scala | 3 ++- src/main/scala/firrtl/util/BackendCompilationUtilities.scala | 3 ++- 6 files changed, 11 insertions(+), 6 deletions(-) (limited to 'src/main') diff --git a/src/main/scala/firrtl/Driver.scala b/src/main/scala/firrtl/Driver.scala index f68a2035..642e7f3f 100644 --- a/src/main/scala/firrtl/Driver.scala +++ b/src/main/scala/firrtl/Driver.scala @@ -14,7 +14,7 @@ import firrtl.stage.{FirrtlExecutionResultView, FirrtlStage} import firrtl.stage.phases.DriverCompatibility import firrtl.options.{StageUtils, Phase, Viewer} import firrtl.options.phases.DeletedWrapper -import firrtl.FileUtils.getText +import firrtl.FileUtils /** @@ -112,7 +112,7 @@ object Driver { JsonProtocol.deserializeTry(file).recoverWith { case jsonException => // Try old protocol if new one fails Try { - val yaml = getText(file).parseYaml + val yaml = FileUtils.getText(file).parseYaml val result = yaml.convertTo[List[LegacyAnnotation]] val msg = s"$file is a YAML file!\n" + (" "*9) + "YAML Annotation files are deprecated! Use JSON" diff --git a/src/main/scala/firrtl/FileUtils.scala b/src/main/scala/firrtl/FileUtils.scala index b859073d..74919f20 100644 --- a/src/main/scala/firrtl/FileUtils.scala +++ b/src/main/scala/firrtl/FileUtils.scala @@ -149,6 +149,7 @@ object FileUtils { def getLinesResource(resourceName: String): Seq[String] = { val inputStream = getClass.getResourceAsStream(resourceName) val text = io.Source.fromInputStream(inputStream).getLines().toSeq + text.length // This forces lazy buffer to reify, please suggest a better solution inputStream.close() text } diff --git a/src/main/scala/firrtl/options/phases/GetIncludes.scala b/src/main/scala/firrtl/options/phases/GetIncludes.scala index 9e198c61..cb9bb840 100644 --- a/src/main/scala/firrtl/options/phases/GetIncludes.scala +++ b/src/main/scala/firrtl/options/phases/GetIncludes.scala @@ -8,6 +8,7 @@ import firrtl.AnnotationSeq import firrtl.annotations.{AnnotationFileNotFoundException, JsonProtocol, LegacyAnnotation} import firrtl.annotations.AnnotationYamlProtocol._ import firrtl.options.{InputAnnotationFileAnnotation, Phase, StageUtils} +import firrtl.FileUtils import java.io.File @@ -27,7 +28,7 @@ class GetIncludes extends Phase { JsonProtocol.deserializeTry(file).recoverWith { case jsonException => // Try old protocol if new one fails Try { - val yaml = io.Source.fromFile(file).getLines().mkString("\n").parseYaml + val yaml = FileUtils.getText(file).parseYaml val result = yaml.convertTo[List[LegacyAnnotation]] val msg = s"$file is a YAML file!\n" + (" "*9) + "YAML Annotation files are deprecated! Use JSON" StageUtils.dramaticWarning(msg) diff --git a/src/main/scala/firrtl/passes/memlib/YamlUtils.scala b/src/main/scala/firrtl/passes/memlib/YamlUtils.scala index eab1fe37..a43adfe2 100644 --- a/src/main/scala/firrtl/passes/memlib/YamlUtils.scala +++ b/src/main/scala/firrtl/passes/memlib/YamlUtils.scala @@ -4,6 +4,7 @@ package firrtl.passes package memlib import net.jcazevedo.moultingyaml._ import java.io.{CharArrayWriter, File, PrintWriter} +import firrtl.FileUtils object CustomYAMLProtocol extends DefaultYamlProtocol { @@ -23,7 +24,7 @@ case class Config(pin: Pin, source: Source, top: Top) class YamlFileReader(file: String) { def parse[A](implicit reader: YamlReader[A]) : Seq[A] = { if (new File(file).exists) { - val yamlString = scala.io.Source.fromFile(file).getLines.mkString("\n") + val yamlString = FileUtils.getText(file) yamlString.parseYamls flatMap (x => try Some(reader read x) catch { case e: Exception => None } diff --git a/src/main/scala/firrtl/stage/phases/DriverCompatibility.scala b/src/main/scala/firrtl/stage/phases/DriverCompatibility.scala index f0f1c067..665861ef 100644 --- a/src/main/scala/firrtl/stage/phases/DriverCompatibility.scala +++ b/src/main/scala/firrtl/stage/phases/DriverCompatibility.scala @@ -6,6 +6,7 @@ import firrtl.stage._ import firrtl.{AnnotationSeq, EmitAllModulesAnnotation, EmitCircuitAnnotation, FirrtlExecutionResult, Parser} import firrtl.annotations.NoTargetAnnotation +import firrtl.FileUtils import firrtl.proto.FromProto import firrtl.options.{InputAnnotationFileAnnotation, OptionsException, Phase, StageOptions, StageUtils} @@ -89,7 +90,7 @@ object DriverCompatibility { annotations.collectFirst{ case FirrtlFileAnnotation(f) => FirrtlStageUtils.getFileExtension(f) match { case ProtoBufFile => FromProto.fromFile(f).main - case FirrtlFile => Parser.parse(io.Source.fromFile(f).getLines().mkString("\n")).main } } ))) + case FirrtlFile => Parser.parse(FileUtils.getText(f)).main } } ))) /** Determine the target directory with the following precedence (highest to lowest): * - Explicitly from the user-specified [[firrtl.options.TargetDirAnnotation TargetDirAnnotation]] diff --git a/src/main/scala/firrtl/util/BackendCompilationUtilities.scala b/src/main/scala/firrtl/util/BackendCompilationUtilities.scala index e0341bf1..beb44ad0 100644 --- a/src/main/scala/firrtl/util/BackendCompilationUtilities.scala +++ b/src/main/scala/firrtl/util/BackendCompilationUtilities.scala @@ -7,6 +7,7 @@ import java.nio.file.Files import java.text.SimpleDateFormat import java.util.Calendar +import firrtl.FileUtils import scala.sys.process.{ProcessBuilder, ProcessLogger, _} @@ -121,7 +122,7 @@ trait BackendCompilationUtilities { // Build a set of canonical file paths to use as a filter to exclude already included additional Verilog sources. val blackBoxHelperFiles: Set[String] = { if(list_file.exists()) { - io.Source.fromFile(list_file).getLines.toSet + FileUtils.getLines(list_file).toSet } else { Set.empty -- cgit v1.2.3 From 2bf399c240938ba51069348f986fa5d65135a808 Mon Sep 17 00:00:00 2001 From: chick Date: Thu, 1 Aug 2019 17:04:45 -0700 Subject: Followup to PR #1142 - use scala.io.Source instead of io.Source - .toList cleaner way to force stream to be read. - clear old commented out code in ProtoBufSpec --- src/main/scala/firrtl/FileUtils.scala | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) (limited to 'src/main') diff --git a/src/main/scala/firrtl/FileUtils.scala b/src/main/scala/firrtl/FileUtils.scala index 74919f20..1007eb2a 100644 --- a/src/main/scala/firrtl/FileUtils.scala +++ b/src/main/scala/firrtl/FileUtils.scala @@ -98,7 +98,7 @@ object FileUtils { * @param fileName The file to read */ def getLines(fileName: String): Seq[String] = { - val source = io.Source.fromFile(fileName) + val source = scala.io.Source.fromFile(fileName) val lines = source.getLines() source.close() lines.toSeq @@ -110,7 +110,7 @@ object FileUtils { * @param file a java File to be read */ def getLines(file: File): Seq[String] = { - val source = io.Source.fromFile(file) + val source = scala.io.Source.fromFile(file) val lines = source.getLines() source.close() lines.toSeq @@ -122,7 +122,7 @@ object FileUtils { * @param fileName The file to read */ def getText(fileName: String): String = { - val source = io.Source.fromFile(fileName) + val source = scala.io.Source.fromFile(fileName) val text = source.mkString source.close() text @@ -134,7 +134,7 @@ object FileUtils { * @param file a java File to be read */ def getText(file: File): String = { - val source = io.Source.fromFile(file) + val source = scala.io.Source.fromFile(file) val text = source.mkString source.close() text @@ -148,8 +148,9 @@ object FileUtils { */ def getLinesResource(resourceName: String): Seq[String] = { val inputStream = getClass.getResourceAsStream(resourceName) - val text = io.Source.fromInputStream(inputStream).getLines().toSeq - text.length // This forces lazy buffer to reify, please suggest a better solution + // the .toList at the end is critical to force stream to be read. + // Without it the lazy evaluation can cause failure in MultiThreadingSpec + val text = scala.io.Source.fromInputStream(inputStream).getLines().toList inputStream.close() text } @@ -162,7 +163,7 @@ object FileUtils { */ def getTextResource(resourceName: String): String = { val inputStream = getClass.getResourceAsStream(resourceName) - val text = io.Source.fromInputStream(inputStream).mkString + val text = scala.io.Source.fromInputStream(inputStream).mkString inputStream.close() text } -- cgit v1.2.3