From 99574e2b9dd01c85367550e8a848f03ff1ea674f Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Sun, 10 May 2020 18:12:18 +0200 Subject: [PATCH] Avoid unnecessary file IO when computing positions The exists call introduced in #8883 and #8897 lead to a slowdown noticeable in benchmarks. Replace them by a check on the content length, since SourceFile caches its content this should avoid any unnecessary IO operation. This required changing the way SourceFile handles empty files to have it return an empty string instead of crashing. --- compiler/src/dotty/tools/dotc/util/SourceFile.scala | 9 ++++++++- .../src/dotty/tools/dotc/util/SourcePosition.scala | 10 +++++----- sbt-bridge/src/xsbt/DelegatingReporter.java | 8 ++++---- 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/util/SourceFile.scala b/compiler/src/dotty/tools/dotc/util/SourceFile.scala index c4aff62cf707..5922b9edb0e6 100644 --- a/compiler/src/dotty/tools/dotc/util/SourceFile.scala +++ b/compiler/src/dotty/tools/dotc/util/SourceFile.scala @@ -52,7 +52,14 @@ class SourceFile(val file: AbstractFile, computeContent: => Array[Char]) extends def maybeIncomplete: Boolean = _maybeInComplete - def this(file: AbstractFile, codec: Codec) = this(file, new String(file.toByteArray, codec.charSet).toCharArray) + def this(file: AbstractFile, codec: Codec) = + // It would be cleaner to check if the file exists instead of catching + // an exception, but it turns out that Files.exists is remarkably slow, + // at least on Java 8 (https://rules.sonarsource.com/java/tag/performance/RSPEC-3725), + // this is significant enough to show up in our benchmarks. + this(file, + try new String(file.toByteArray, codec.charSet).toCharArray + catch case _: java.nio.file.NoSuchFileException => Array[Char]()) /** Tab increment; can be overridden */ def tabInc: Int = 8 diff --git a/compiler/src/dotty/tools/dotc/util/SourcePosition.scala b/compiler/src/dotty/tools/dotc/util/SourcePosition.scala index db489a3d172b..9fa949fd2cdf 100644 --- a/compiler/src/dotty/tools/dotc/util/SourcePosition.scala +++ b/compiler/src/dotty/tools/dotc/util/SourcePosition.scala @@ -42,16 +42,16 @@ extends interfaces.SourcePosition with Showable { def beforeAndAfterPoint: (List[Int], List[Int]) = lineOffsets.partition(_ <= point) - def column: Int = if (source.file.exists) source.column(point) else -1 + def column: Int = if (source.content().length != 0) source.column(point) else -1 def start: Int = span.start - def startLine: Int = if (source.file.exists) source.offsetToLine(start) else -1 - def startColumn: Int = if (source.file.exists) source.column(start) else -1 + def startLine: Int = if (source.content().length != 0) source.offsetToLine(start) else -1 + def startColumn: Int = if (source.content().length != 0) source.column(start) else -1 def startColumnPadding: String = source.startColumnPadding(start) def end: Int = span.end - def endLine: Int = if (source.file.exists) source.offsetToLine(end) else -1 - def endColumn: Int = if (source.file.exists) source.column(end) else -1 + def endLine: Int = if (source.content().length != 0) source.offsetToLine(end) else -1 + def endColumn: Int = if (source.content().length != 0) source.column(end) else -1 def withOuter(outer: SourcePosition): SourcePosition = SourcePosition(source, span, outer) def withSpan(range: Span) = SourcePosition(source, range, outer) diff --git a/sbt-bridge/src/xsbt/DelegatingReporter.java b/sbt-bridge/src/xsbt/DelegatingReporter.java index 4fe7cd40d95b..c90e5882d69c 100644 --- a/sbt-bridge/src/xsbt/DelegatingReporter.java +++ b/sbt-bridge/src/xsbt/DelegatingReporter.java @@ -91,7 +91,7 @@ public Optional sourceFile() { return Optional.ofNullable(src.file().file()); } public Optional line() { - if (!src.file().exists()) + if (src.content().length == 0) return Optional.empty(); int line = pos.line() + 1; @@ -101,7 +101,7 @@ public Optional line() { return Optional.of(line); } public String lineContent() { - if (!src.file().exists()) + if (src.content().length == 0) return ""; String line = pos.lineContent(); @@ -116,13 +116,13 @@ public Optional offset() { return Optional.of(pos.point()); } public Optional pointer() { - if (!src.file().exists()) + if (src.content().length == 0) return Optional.empty(); return Optional.of(pos.point() - src.startOfLine(pos.point())); } public Optional pointerSpace() { - if (!src.file().exists()) + if (src.content().length == 0) return Optional.empty(); String lineContent = this.lineContent();