Skip to content

Commit

Permalink
Merge pull request #1939 from lwronski/performance-tweaks
Browse files Browse the repository at this point in the history
Performance tweaks
  • Loading branch information
lwronski committed Mar 28, 2023
2 parents e270476 + 06a6393 commit 37af4a6
Show file tree
Hide file tree
Showing 27 changed files with 71 additions and 320 deletions.
4 changes: 2 additions & 2 deletions modules/build/src/main/scala/scala/build/Build.scala
Original file line number Diff line number Diff line change
Expand Up @@ -801,6 +801,7 @@ object Build {
compilerJvmVersionOpt: Option[Positioned[Int]],
scope: Scope,
logger: Logger,
artifacts: Artifacts,
maybeRecoverOnError: BuildException => Option[BuildException] = e => Some(e)
): Either[BuildException, Project] = either {

Expand All @@ -809,8 +810,6 @@ object Build {
val classesDir0 = classesDir(inputs.workspace, inputs.projectName, scope)
val scaladocDir = classesDir(inputs.workspace, inputs.projectName, scope, suffix = "-doc")

val artifacts = value(options.artifacts(logger, scope, maybeRecoverOnError))

val generateSemanticDbs = options.scalaOptions.generateSemanticDbs.getOrElse(false)

val releaseFlagVersion = releaseFlag(options, compilerJvmVersionOpt, logger).map(_.toString)
Expand Down Expand Up @@ -1007,6 +1006,7 @@ object Build {
compilerJvmVersionOpt,
scope,
logger,
artifacts,
maybeRecoverOnError
)
}
Expand Down
4 changes: 4 additions & 0 deletions modules/build/src/main/scala/scala/build/Directories.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ package scala.build

import coursier.cache.shaded.dirs.{GetWinDirs, ProjectDirectories}

import scala.build.errors.ConfigDbException
import scala.build.internal.JniGetWinDirs
import scala.cli.config.ConfigDb
import scala.util.Properties

trait Directories {
Expand All @@ -21,6 +23,8 @@ trait Directories {
.filter(_.trim.nonEmpty)
.map(os.Path(_, os.pwd))
.getOrElse(secretsDir / Directories.defaultDbFileName)

lazy val configDb = ConfigDb.open(dbPath.toNIO).left.map(ConfigDbException(_))
}

object Directories {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,13 +79,12 @@ case object MarkdownPreprocessor extends Preprocessor {
.map { wrappedMarkdown =>
val processingOutput: ProcessingOutput =
value {
ScalaPreprocessor.process(
ScalaPreprocessor.processSources(
content = wrappedMarkdown.code,
extractedDirectives = wrappedMarkdown.directives,
path = reportingPath,
scopeRoot = scopePath / os.up,
logger = logger,
maybeRecoverOnError = maybeRecoverOnError,
allowRestrictedFeatures = allowRestrictedFeatures,
suppressWarningOptions = suppressWarningOptions
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,32 +205,30 @@ case object ScalaPreprocessor extends Preprocessor {
suppressWarningOptions: SuppressWarningOptions
): Either[BuildException, Option[ProcessingOutput]] = either {
val (contentWithNoShebang, _) = SheBang.ignoreSheBangLines(content)
val extractedDirectives = value(ExtractedDirectives.from(
val extractedDirectives: ExtractedDirectives = value(ExtractedDirectives.from(
contentWithNoShebang.toCharArray,
path,
logger,
scopeRoot,
maybeRecoverOnError
))
value(process(
value(processSources(
content,
extractedDirectives,
path,
scopeRoot,
logger,
maybeRecoverOnError,
allowRestrictedFeatures,
suppressWarningOptions
))
}

def process(
def processSources(
content: String,
extractedDirectives: ExtractedDirectives,
path: Either[String, os.Path],
scopeRoot: ScopePath,
logger: Logger,
maybeRecoverOnError: BuildException => Option[BuildException],
allowRestrictedFeatures: Boolean,
suppressWarningOptions: SuppressWarningOptions
): Either[BuildException, Option[ProcessingOutput]] = either {
Expand All @@ -246,13 +244,6 @@ case object ScalaPreprocessor extends Preprocessor {
suppressWarningOptions
))

value {
checkForAmmoniteImports(
afterStrictUsing.strippedContent.getOrElse(content0),
path
)
}

if (afterStrictUsing.isEmpty) None
else {
val allRequirements = Seq(afterStrictUsing.globalReqs)
Expand All @@ -274,70 +265,6 @@ case object ScalaPreprocessor extends Preprocessor {
}
}

private def checkForAmmoniteImports(
content: String,
path: Either[String, os.Path]
): Either[BuildException, Unit] = {

import fastparse.*

import scala.build.internal.ScalaParse.*

val res = parse(content, Header(_))

val indicesOrFailingIdx0 = res.fold((_, idx, _) => Left(idx), (value, _) => Right(value))

val indicesOrErrorMsg = indicesOrFailingIdx0 match {
case Left(failingIdx) =>
val newCode = content.take(failingIdx)
val res1 = parse(newCode, Header(_))
res1 match {
case f: Parsed.Failure =>
val msg = formatFastparseError(Util.printablePath(path), content, f)
Left(msg)
case s: Parsed.Success[Seq[(Int, Int)]] =>
Right(s.value)
}
case Right(ind) =>
Right(ind)
}

// TODO Report error if indicesOrErrorMsg.isLeft?

val importTrees = indicesOrErrorMsg
.toSeq
.iterator
.flatMap(_.iterator)
.flatMap {
case (start, end) =>
val code = content.substring(start, end) // .trim // meh
val importRes = parse(code, ImportSplitter(_))
importRes.fold((_, _, _) => Iterator.empty, (trees, _) => trees.iterator).map { tree =>
(start, tree.copy(start = start + tree.start, end = start + tree.end))
}
}
.toVector

val dependencyTrees = importTrees.filter { case (_, t) =>
val firstSegmentOpt = t.prefix.headOption
(firstSegmentOpt.contains("$ivy") || firstSegmentOpt.contains("$dep")) &&
t.prefix.lengthCompare(1) > 0
}

if (dependencyTrees.nonEmpty) {
val toFilePos = Position.Raw.filePos(path, content)
val exceptions = for {
(importStart, t) <- dependencyTrees
pos = toFilePos(Position.Raw(importStart, t.end))
dep = t.prefix.drop(1).mkString(".")
newImportText = s"//> using dep \"$dep\""
} yield new UnsupportedAmmoniteImportError(Seq(pos), newImportText)

Left(CompositeBuildException(exceptions))
}
else Right(())
}

private def processStrictUsing(
content: String,
extractedDirectives: ExtractedDirectives,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,46 +84,6 @@ class ActionableDiagnosticTests extends munit.FunSuite {
}
}

test("error on ammonite imports") {
val dependencyOsLib = "com.lihaoyi::os-lib:0.7.8"
val dependencyUpickleLib = "com.lihaoyi::upickle:1.4.0"
val ivyImport = s"import $$ivy.`$dependencyOsLib`"
val depImport = s"import $$dep.`$dependencyUpickleLib`"
val testInputs = TestInputs(
os.rel / "Foo.scala" ->
s"""$ivyImport
|$depImport
|
|object Hello extends App {
| println("Hello")
|}
|""".stripMargin
)
testInputs.withBuild(baseOptions, buildThreads, None, actionableDiagnostics = true) {
(_, _, maybeBuild) =>
expect(maybeBuild.isLeft)
val exceptions = maybeBuild match {
case Left(c: CompositeBuildException) => c.exceptions
case _ => Seq()
}

expect(exceptions.length == 2)
expect(exceptions.forall(_.isInstanceOf[UnsupportedAmmoniteImportError]))

expect(exceptions.head.textEdit.get.newText == s"//> using dep \"$dependencyOsLib\"")
expect(
exceptions.tail.head.textEdit.get.newText == s"//> using dep \"$dependencyUpickleLib\""
)

val filePositions = exceptions.flatMap(_.positions.collect {
case File(_, startPos, endPos) => (startPos, endPos)
})

expect(filePositions.head == ((0, 0), (0, ivyImport.length)))
expect(filePositions.tail.head == ((1, 0), (1, depImport.length)))
}
}

test("actionable actions suggest update only to stable version") {
val testInputs = TestInputs(
os.rel / "Foo.scala" ->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,17 +84,19 @@ class BuildProjectTests extends munit.FunSuite {
)
)

val inputs = Inputs.empty("project")
val sources = Sources(Nil, Nil, None, Nil, options)
val logger = new LoggerMock()
val inputs = Inputs.empty("project")
val sources = Sources(Nil, Nil, None, Nil, options)
val logger = new LoggerMock()
val artifacts = options.artifacts(logger, Scope.Test).orThrow
val res = Build.buildProject(
inputs,
sources,
Nil,
options,
Some(Positioned(bloopJavaPath, bloopJvmVersion)),
Scope.Test,
logger
logger,
artifacts
)

val scalaCompilerOptions = res.fold(throw _, identity)
Expand Down Expand Up @@ -169,12 +171,13 @@ class BuildProjectTests extends munit.FunSuite {
LocalRepo.localRepo(scala.build.Directories.default().localRepoDir)
)
)
val inputs = Inputs.empty("project")
val sources = Sources(Nil, Nil, None, Nil, options)
val logger = new LoggerMock()
val inputs = Inputs.empty("project")
val sources = Sources(Nil, Nil, None, Nil, options)
val logger = new LoggerMock()
val artifacts = options.artifacts(logger, Scope.Test).orThrow

val project =
Build.buildProject(inputs, sources, Nil, options, None, Scope.Main, logger).orThrow
Build.buildProject(inputs, sources, Nil, options, None, Scope.Main, logger, artifacts).orThrow

expect(project.workspace == inputs.workspace)
}
Expand Down
26 changes: 0 additions & 26 deletions modules/build/src/test/scala/scala/build/tests/BuildTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -250,32 +250,6 @@ abstract class BuildTests(server: Boolean) extends munit.FunSuite {
simpleNativeTest()
}

test("should fail for ammonite imports - $ivy") {
val testInputs = TestInputs(
os.rel / "simple.sc" ->
"""import $ivy.`com.lihaoyi::geny:0.6.5`
|import geny.Generator
|val g = Generator("Hel", "lo")
|println(g.mkString)
|""".stripMargin
)
testInputs.withBuild(defaultOptions, buildThreads, bloopConfigOpt) { (_, _, maybeBuild) =>
expect(maybeBuild.isLeft)
}
}
test("should fail for ammonite imports - $dep") {
val testInputs = TestInputs(
os.rel / "simple.sc" ->
"""import $dep.`com.lihaoyi::geny:0.6.5`
|import geny.Generator
|val g = Generator("Hel", "lo")
|println(g.mkString)
|""".stripMargin
)
testInputs.withBuild(defaultOptions, buildThreads, bloopConfigOpt) { (_, _, maybeBuild) =>
expect(maybeBuild.isLeft)
}
}
test("dependencies - using") {
val testInputs = TestInputs(
os.rel / "simple.sc" ->
Expand Down
21 changes: 0 additions & 21 deletions modules/build/src/test/scala/scala/build/tests/SourcesTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -245,27 +245,6 @@ class SourcesTests extends munit.FunSuite {
}
}

test("should fail for ammonite imports in .sc - $ivy") {
val testInputs = TestInputs(
os.rel / "something.sc" ->
"""import $ivy.`org1:name1:1.1`
|import $ivy.`org2::name2:2.2`
|import $ivy.`org3:::name3:3.3`
|import scala.collection.mutable
|
|def a = 1
|""".stripMargin
)
testInputs.withInputs { (_, inputs) =>
expect(CrossSources.forInputs(
inputs,
preprocessors,
TestLogger(),
SuppressWarningOptions()
).isLeft)
}
}

test("should skip SheBang in .sc and .scala") {
val testInputs = TestInputs(
os.rel / "something1.sc" ->
Expand Down
3 changes: 2 additions & 1 deletion modules/cli/src/main/scala/scala/cli/ScalaCli.scala
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import scala.cli.config.{ConfigDb, Keys}
import scala.cli.internal.Argv0
import scala.cli.launcher.{LauncherCli, LauncherOptions}
import scala.cli.publish.BouncycastleSignerMaker
import scala.cli.util.ConfigDbUtils
import scala.util.Properties

object ScalaCli {
Expand All @@ -36,7 +37,7 @@ object ScalaCli {
private val scalaCliBinaryName = "scala-cli"
private var isSipScala = {
lazy val isPowerConfigDb = for {
configDb <- ConfigDb.open(Directories.directories.dbPath.toNIO).toOption
configDb <- ConfigDbUtils.configDb.toOption
powerEntry <- configDb.get(Keys.power).toOption
power <- powerEntry
} yield power
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,11 +206,11 @@ abstract class ScalaCommand[T <: HasGlobalOptions](implicit myParser: Parser[T],
def maybePrintToolsHelp(options: T, buildOptions: BuildOptions): Unit =
for {
shared <- sharedOptions(options)
if shared.helpGroups.helpScaladoc || shared.helpGroups.helpRepl || shared.helpGroups.helpScalafmt
logger = shared.logger
artifacts <- buildOptions.artifacts(logger, Scope.Main).toOption
scalaArtifacts <- artifacts.scalaOpt
scalaParams = scalaArtifacts.params
if shared.helpGroups.helpScaladoc || shared.helpGroups.helpRepl || shared.helpGroups.helpScalafmt
} {
val exitCode: Either[BuildException, Int] = either {
val (classPath: Seq[os.Path], mainClass: String) =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ object Bsp extends ScalaCommand[BspOptions] {

val inputs = argsToInputs(args.all).orExit(logger)
CurrentParams.workspaceOpt = Some(inputs.workspace)
val configDb = options.shared.configDb.orExit(logger)
val actionableDiagnostics =
options.shared.logging.verbosityOptions.actions

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import scala.cli.commands.util.BuildCommandHelpers
import scala.cli.commands.{CommandUtils, ScalaCommand, SpecificationLevel, WatchUtil}
import scala.cli.config.{ConfigDb, Keys}
import scala.cli.util.ArgHelpers.*

import scala.cli.util.ConfigDbUtils
object Compile extends ScalaCommand[CompileOptions] with BuildCommandHelpers {
override def group: String = HelpCommandGroup.Main.toString

Expand Down Expand Up @@ -92,7 +92,7 @@ object Compile extends ScalaCommand[CompileOptions] with BuildCommandHelpers {
val threads = BuildThreads.create()

val compilerMaker = options.shared.compilerMaker(threads).orExit(logger)
val configDb = options.shared.configDb.orExit(logger)
val configDb = ConfigDbUtils.configDb.orExit(logger)
val actionableDiagnostics =
options.shared.logging.verbosityOptions.actions.orElse(
configDb.get(Keys.actions).getOrElse(None)
Expand Down

0 comments on commit 37af4a6

Please sign in to comment.