The old sbt launcher uses jansi 1.11, which is incompatible with jline3.
To work around this, we can use the jna terminal implementation for the
jline system terminal. This commit also switches to using the jline
TerminalBuilder for all system terminals except for the windows system
terminal with the thin client. The jline terminal builder uses
reflection that is difficult to make work with the thin client and it is
much easier to just manually construct the thin client. This is only
necessary for windows because on posix the thin client will fall back to
an implementation that shells out for stty commands.
The thin client needs to do its own success reporting because in batch
mode it's possible for the task to exit before success is logged by the
server. If the server also prints success, there can be double printing.
Unfortunately, the Prompt.Batch check is not reliable because MainLoop
will change the prompt to Running during task evaluation. The
interactive flag is set in the NetworkChannel when the client explicitly
registers itself as an interactive session, so this should be more
reliable.
I noticed in CI that sometimes the client tests exit with an interrupted
exception printed. I tracked it down the exception to the call to
getExec, which delegateds to CommandExchange.blockUntilNextExec.
In a continuous build in sbt 1.4.0-RC1, if the user enters an invalid
option, it causes the input thread to exit which means the watch would
no longer accept input commands (including <enter> to exit). This fixes
that behavior.
In sbt 1.4.0-RC1, if a user ran `sbt console`, the progress lines would
be printed after they had entered the console. This was because the
prompt state was incorrect. To get the prompt in the correct state, we
initialize the prompt to batch and then switch to pending when either
sbt enters the shell or the network client attaches in interactive mode.
We also will now immediately print progress as soon as we enter a skip
task to clear out the progress lines and display the warning about a
running task if there is another client connected while the task is
running.
The clean task was previously deleting the contents of directories that
were symlinked into the target directory. This was an oversight because
it never occurred to me that users might symlink a directory whose
contents they did not want deleted into the target directory.
Fixes https://github.com/sbt/sbt/issues/5822
Currently the entire shell gets stuck when there's a compilation error with pipelining.
This at least returns to sbt shell.
Together with https://github.com/sbt/zinc/pull/920 this fixes most of the mixed pipelining issues.
1. Previous values are carried from `compileScalaBackend` in `compileJavaTask`.
2. `compileJava / compileOptions ` now uses `compile / compileOptions` to avoid unintentional change of javac or scalac options.
3. Hooks up early compile analysis store.
Ref https://github.com/sbt/sbt/issues/5665
This adds `--server` command that is immediately filtered out in Main.scala.
The purpose of `--server` is so we can invoke thin client from `sbt` script at some point in the future when Bash script can parse `project/build.properties`.
`sbtn` would need to call `sbt` again to start the server, and at that point the shell script would need to actually invoke the server. The intent of `--server` is to be used as the tie breaker.
Also build users may want to sometimes call `sbt --server`.
I introduced the terminalShellPrompt so that we could generate a prompt
that was colored only if the terminal supported color. Rather than
expose the terminal implementation detail, we can just use a boolean
flag that toggles whether or not color is enabled and sbt can pass in
the value of terminal.isColorEnabled into the function.
It shouldn't be the case that a RejectedExecutionException is thrown
by TaskProgress. If that assumption is violated, log the exception but
don't crash sbt.
The play plugin seems to do out of band task evaluation on a stale State
object in the `run` task. As a result, when sbt tries to schedule tasks
to run, they tried to register the work with a closed TaskProgress
instance. There was no guard against this and it ended up causing a
RejectedExecutionException.
sbt 1.4.0 generates the shell prompt using the terminal properties for
the specific terminal for which the prompt is rendered. The mechanism
for doing this broke the prompt for projects that overrode the
shellPrompt key, notably the play plugin. After this change, the play
custom prompt is correctly rendered with 1.4.0-SNAPSHOT.
The RelayAppender should not log directly to console out since it is
supposed to be relaying json log messages to connected clients. This was
manifesting as double printing on some success messages.
Running publishLocal in the zinc project can cause gc thrashing with the
default parallelgc collector using jdk8 on my laptop. If I switch to
G1GC, it does not thrash even if I leave the heap the same size.
The java GarbageCollectorMXBean.getCollectionTime returns the cumulative
amount of time the collector has run during the jvm session. The GC
monitor is tracking how much time has been spent in garbage collection
during each task evaluation run. In order for this calculation to work
correctly, it is necessary to set the initial elapsed time to the bean's
current collection time when we create the gc monitor. Without doing
this, we can get completely incorrect results that are reporting based
on the total gc time for the entire process, not just in the last 10
seconds.
Should fix https://github.com/sbt/sbt/issues/5818
It is not uncommon in large projects for the jvm to silently be running
frequent full gcs in the background. This can slow progress to a crawl.
Usually the fix is to bump the -Xmx parameter but if the users do not
realize that their tasks are slow because of gc thrashing, they may not
think to do that. This PR adds a monitor that hooks into the jvm's event
notification system to keep track of how much time is spent in GC. If
the ratio of the amount of time in gc to the total elapsed time exceeds
some threshold, we emit a warning.
I was motivated to do this because publishLocal can take forever in the
zinc project because a 1G heap isn't big enough.
These tasks show up during task progress and they clutter up the
display. Since my understanding is that both of these tasks are more or
less just waiting for other work to complete, I don't think they are
helpful for debugging.
The zinc scripted project depends on all of the compiler bridges. As a
result the introduction of the strict scala binary version check in
f8139da192 broke zinc scripted. This
commit reverts to the old behavior in the non scala sandwich case.
I also switched to a for comprehension instead of a pattern match
because this is a rare case where I think it made the code significantly
more readable.
While in a continuous build, when the user enters ctrl+c into the sbt
server console (not a thin client connection) when sbt has been launched
in interactive mode, the server exits. This commit makes it so that
instead we just cancel the watch. As a result, if sbt was started in
batch mode, e.g. `sbt ~compile`, ctrl+c will still exit sbt but in
interactive mode ctrl+c will take the user back to the shell.
I occassionally end up in a state where watch input does not seem to be
read. To rule out the possibility that the background thread that reads
input has not successfully started, this commit makes it so that we
block until the thread signals that it has started via a CountDownLatch.
The diff is superficially big because of an indentation change at the
bottom.
The sbt Server is initialized with a callback onIncomingSocket. That
callback was created in CommandExchange and held references to a build
structure and a state. Neither the state nor structure would ever go out
of scope so they effectively leaked. It is possible for each
NetworkChannel to access a recent instance of state through the
CommandExchange.withState method. Using this, we can eliminate the
references to state and build structure in the onIncomingSocket
callback. In the sbt project, this reduced the memory utilization by
about 50mb on startup.
On linux and mac, entering ctrl+c will automatically kill any forked
processes that were created by the sbt server because sigint is
automatically forwarded to the child process. This is not the case on
windows where it is necessary to forcibly kill these processes.
The intellij import currentlly works by forking an sbt process and
writing command input through the process input stream. To make this
work, we need the SimpleTerminal (which is used when sbt is run with
-Dsbt.log.noformat=true) to be able to read input.
Attaching the input to the simple terminal caused watch tests to fail on
windows. This can be fixed by checking if the byte read from the input
stream is -1 and ignoring it if so.
The sbt.log.noformat parameter should be treated very similarly to
sbt.io.virtual. When it is true, we should just use the raw io streams
for the process. This came up because of
https://github.com/sbt/sbt/issues/5784 which reported that intellij
imports were not working and that ansi control characters were being
written to the output.
There can be race conditions where we try to interrupt and join a ui
thread before it becomes interruptible by blockign on a queue. To
workaround this, we can add the JoinThread class which adds an
extension method Thread.joinFor that takes a FiniteDuration parameter.
This variant of join will repeatedly interrupt and attempt to join the
thread for up to 10 milliseconds before retrying until the limit is
reached. If the limit is reached, we print a noisy error to the console.
I'm not 100% sure if we are leaking threads in the latest sbt version
but this gives me more piece of mind that either we are always
successfully joining the threads or we will be alerted if the joining
fails.
Although log manager is in the internal package, akka uses it and it
seems worth preserving binary compatibility in their build, at least for
sbt 1.4.0-M2. Once the community build is passing, we can consider
reverting this.
```
[info] welcome to sbt 1.4.0-SNAPSHOT (AdoptOpenJDK Java 1.8.0_232)
[info] loading settings for project global-plugins from ...
[info] loading global plugins from ...
[info] loading project definition from /private/tmp/hello/project
[info] loading settings for project root from build.sbt ...
[info] set current project to hello (in build file:/private/tmp/hello/)
[info]
[info] Here are some highlights of this release:
[info] - Build server protocol (BSP) support
[info] - sbtn: a native thin client for sbt
[info] - VirtualFile + RemoteCache: caches build artifacts across different machines
[info] - Incremental build pipelining. Try it using `ThisBuild / usePipelining := true`.
[info] See http://eed3si9n.com/sbt-1.4.0-beta for full release notes.
[info] Hide the banner for this release by running `skipBanner`.
[info] sbt server started at local:///Users/eed3si9n/.sbt/1.0/server/478e6db75688771ddcf1/soc
```
Frequently ctrl+c does not work to cancel the running tasks. This seems
to be because the signal handler is bound to a specific instance of
evaluate task but there may be multiple instances of evaluate task
running at any given time. Shutting down just one of the running engines
does not ensure that task evaluation stops. To work around this, we can
globally store all of the completion services in a weak hash map and
cancel all of them whenever a signal is received. Closing the service,
which happens at the end of task evaluation will remove the service from
the map so hopefully this shouldn't introduce a memory leak.
While dogfooding the latest sbt, I noticed that sometimes the watch
input threads leak. I suspect this happens when a build is immediately
triggered by a file that was modified during compilation. Though I
didn't fully verify this, it's likely that we interrupted the input
reading thread before it actually started reading. When it started
reading after the interrupt, it would block until the user entered
another input character. The result was that the zombie thread would
effectively steal the next character from the input stream which
manifested as the first character being ignored when the user tried to
enter a watch input option. If more than one thread leaked, then it may
take a number of keystrokes before the user regained control.
To fix this, we can ensure that all watch related threads are joined
before we exit watch. To avoid completely blocking the ui, we only try
to join the threads for a second and print an error if the join fails.
This shouldn't be the case so if users see this error, then we need to
fix the bug.
In 0d2b00c7e9, I introduced a setting for
the virtual file defines class cache to avoid ooms coming from zinc
stamping the project jar files. I introduced that cache at the compile
level though rather than global level and crashes were still occurring
in the sbt build. It was very easy to induce a crash on my computer by
running compile a few times, reload and then compile again. After making
the cache global, the crashes went away.
This attempts to delay the initialization of Coursier cache, such that it will not trigger Coursier directory related code if `ThisBuild / useCoursier` or `-Dsbt.coursier` is set to `false`.
With the latest sbt snapshot, the ui would get stuck if the user entered
an empty command. They would be presented with an empty prompt and could
not input any commands. This was caused by the change in
d569abe70a that reset the prompt after a
line was read. I had tried to optimize line reading by ignoring empty
commands in UITask.readline so we wouldn't have to make a new thread.
This optimization wasn't really buying much since it only affects how
quickly the user is reprompted after entering an empty command. Unless a
user is spamming the <enter> key, they shouldn't notice a difference.
This commit adds a few options to supershell:
1. Max items -- sets the max number of tasks to display in the progress
reports. It is pretty hard to read more than a few items in the
progress reports so I set the default limit to 8 and made that
configurable via the superShellMaxTasks parameter. If there are more
than the limit, there is an additional line telling how many additional
tasks are running
2. sleep -- sets how long to sleep between reports. The default is 500ms
to ensure that it updates at least once per second but the previous
value of 100ms is more frequent than necessary
3. threshold -- sets the minimum duration a task has to run before being
printed in the progress reports. The default threshold is increased
from 10ms to 100ms. This introduces a delay of threshold milliseconds
before any progress lines appear and also means that if no tasks ever
exceed the threshold, then no progress is ever displayed.
It turns out that task progress actually introduces a fair bit of
overhead. The biggest issue is that the task progress callbacks block
the Execute main thread. This means that time in those callbacks
delays task evaluation, slowing down sbt. This was not negligible, I was
seeing a lot of the total time of a no-op compile in
https://github.com/jtjeferreira/sbt-multi-module-sample was spent in
TaskProgress callbacks. Prior to these changes, I ran 30 no-op compiles
in that project and the average time was about 570ms. This number got
worse and worse because there were memory leaks in the TaskProgress
object. After these changes, it dropped to 250ms and after jit-ing, it
would drop to about 200ms. I also successfully ran 5000 consecutive
no-op compiles without leaking any memory.
A lot of the overhead of task progress was in adding tasks to the
timings map in AbstractTaskProgress. Tasks were never removed and
ConcurrentHashMap insertion time is proportional to the size of the map
(not sure if it's linear, quadratic or other) which was why sbt actually
got slower and slower the longer it ran. Much of the time was spent
adding tasks to the progress timings.
To fix this, I did something similar to what I did to manage logger
state in https://github.com/jtjeferreira/sbt-multi-module-sample. In
MainLoop, we create a new TaskProgress instance before command
evaluation and clean it up after. Earlier I made TaskProgress an object
to try to ensure there was only one progress thread at a time, and that
introduced the memory leak. In addition to removing the leak, I was able
to improve performance by removing tasks from the timings map when they
completed. Unlike TaskTimings and TaskTraceEvent, we don't care about
tasks that have completed for TaskProgress so it is safe to remove them.
In addition to the memory leaks, I also reworked how the background
threads work. Instead of having one thread that sleeps and prints
progress reports, we now use two single threaded executors. One is a
scheduled executor that is used to schedule progress reports and the
other is the actual thread on which the report is generated. When
progress starts, we schedule a recurring report that is generated every
sleep interval until task evaluation completes. Whenever we add a new
task, if we have haven't previously generated a progress report, we
schedule a report in threshold milliseconds. If the task completes
before the threshold period has elapsed, we just cancel the schedule
report. By doing things this way, we reduce the total number of reports
that are generated. Because reports need to effectively lock System.out,
the less we generate them, the better.
I also modified the internal data structures of AbstractTaskProgress so
that there is a single task map of timings instead of one map for
timings and one for active tasks.
It was a bit tricky to reason about the state of the prompt for a
terminal. To help make things more clear, I reworked things so that the
LineReader always sets the prompt to Pending after it reads a command.
In MainLoop, we cache the prompt value and temporarily set it to Running
while the command is running, which is really how it should have always
been.
In order to make the console task work with scala 2.13 and the thin
client, we need to provide a way for the scala repl to use an sbt
provided jline3 terminal instead of the default terminal typically built
by the repl. We also need to put jline 3 higher up in the classloading
hierarchy to ensure that two versions of jline 3 are not loaded (which
makes it impossible to share the sbt terminal with the scala terminal).
One impact of this change is the decoupling of the version of
jline-terminal used by the in process scala console and the version
of jline-terminal specified by the scala version itself. It is possible
to override this by setting the `useScalaReplJLine` flag to true. When
that is set, the scala REPL will run in a fully isolated classloader. That
will ensure that the versions are consistent. It will, however, for sure
break the thin client and may interfere with the embedded shell ui.
As part of this work, I also discovered that jline 3 Terminal.getSize is
very slow. In jline 2, the terminal attributes were automatically cached with a
timeout of, I think, 1 second so it wasn't a big deal to call
Terminal.getAttributes. The getSize method in jline 3 is not cached and
it shells out to run a tty command. This caused a significant
performance regression in sbt because when progress is enabled, we call
Terminal.getSize whenever we log any messages. I added caching of
getSize at the TerminalImpl level to address this. The timeout is 1
second, which seems responsive enough for most use cases. We could also
move the calculation onto a background thread and have it periodically
updated, but that seems like overkill.
There are cases where if the ui state is changing rapidly, that an
AskUserThread can be created and cancelled in a short time windows. This
could cause problems if the AskUserThread is interrupted during
`LineReader.createReader` which I think can shell out to run some
commands so it is relatively slow. If the thread was interrupted during
the call to `LineReader.createReader` and the interruption was not
handled, then the thread would go into `LineReader.readLine`, which
wouldn't exit until the user pressed enter. This ultimately caused the
ui to break until enter because this zombie line reader would be holding
the lock on the terminal input stream.
Prior to these changes, sbt was leaking large amounts of memory via
log4j appenders. sbt has an unusual use case for log4j because it
creates many ephemeral loggers while also having a global logger that is
supposed to work for the duration of the sbt session. There is a lot of
shared global state in log4j and properly cleaning up the ephemeral task
appenders would break global logging. This commit fixes the behavior by
introducing an alternate logging implementation. Users can still use the
old log4j logging implementation but it will be off by default. The
internal implementation is very simple: it just blocks the current
thread and writes to all of the appenders. Nevertheless, I found the
performance to be roughly identical to that of log4j in my sample
project. As an experiment, I did the appending on a thread pool and got
a significant performance improvement but I'll defer that to a later PR
since parallel io is harder to reason about.
Background: I was testing sbt performance in
https://github.com/jtjeferreira/sbt-multi-module-sample and noticed that
performance rapidly degraded after I ran compile a few times. I took a
heap dump and it became obvious that sbt was leaking console appenders.
Further investigation revealed that all of the leaking appenders in the
project were coming from task streams. This made me think that the fix
would be to track what loggers were created during task evaluation and
clear them out when task evaluation completed. That almost worked except
that log4j has an internal append only data structure containing logger
names. Since we create unique logger names for each run, that internal
data structure grew without bound. It looked like this could be worked
around by creating a new log4j Configuration (where that data structure
was stored) but while creating new configurations with each task runs
did fix the leak, it also broke global logging, which was using a
different configuration. At this point, I decided to write an alternate
implementation of the appender api where I could be sure that the
appenders were cleaned up without breaking global logging.
Implementation: I made ConsoleAppender a trait and made it no longer
extends log4j AbstractAppender. To do this, I had to remove the one
log4j specific method, append(LogEvent). ConsoleAppender now has a
method toLog4J that, in most cases, will return a log4j Appender that is
almost identical to the Appenders that we previously used. To manage
the loggers created during task evaluation, I introduce a new class,
LoggerContext. The LoggerContext determines which logging backend to use
and keeps track of what appenders and loggers have been created. We can
create a fresh LoggerContext before each task evaluation and clear it
out, cleaning up all of its resources after task evaluation concludes.
In order to make this work, there were many places where we need to
either pass in a LoggerContext or create a new one. The main magic is
happening in the `next(State)` method in Main. This is where we create a
new LoggerContext prior to command evaluation and clean it up after the
evaluation completes.
Users can toggle log4j using the new useLog4J key. They also can set the
system property, sbt.log.uselog4j. The global logger will use the sbt
internal implementation unless the system property is set.
There are a fairly significant number of mima issues since I changed the
type of ConsoleAppender. All of the mima changes were in the
sbt.internal package so I think this should be ok.
Effects: the memory leaks are gone. I successfully ran 5000 no-op
compiles in the sbt-multi-module-sample above with no degradation of
performace. There was a noticeable degradation after 30 no-op compiles
before.
During the refactor, I had to work on TestLogger and in doing so I also
fixed https://github.com/sbt/sbt/issues/4480.
This also should fix https://github.com/sbt/sbt/issues/4773
Zinc frequently needs to check the library classpath to ensure that
class names are defined in a given jar. There is a cost to looking up
the class names in the jar so it's a benefit to cache this across runs
so that we don't have to redo the same work every time. More
importantly, in testing with the latest sbt HEAD, I found that sbt would
crash fairly frequently because it ran out of direct memory, which is
used by nio to read and write to native memory without copying. The
direct memory area is shared with the java heap and if it reaches the
limit, the jvm crashes hard as though kill -9 was invoked. After caching
the entries, I stopped seeing crashes.
Rather than relying on a command, I realized it makes more sense to
explicitly set the terminal for the calling channel in MainLoop. By
doing it this way, we can also ensure that we always reset it to the
previous value.
Using the scala reflect library always introduces significant
classloading overhead. We can eliminate the classloading overhead by
generating StringTypeTags at compile time instead.
This sped up average project loading time by a few hundred milliseconds
on my computer. The ManagedLoggedReporter in zinc is still using the
type tag based apis but after the next sbt release, we can upgrade the
zinc apis. We also could consider breaking binary compatibility.
sbt depends on scalacache (which hasn't been updated in about a year)
and we really don't need the functionality provided by scalacache. In
fact, the java api is somewhat easier to work with for our use case. The
motivation is that scalacache uses slf4j for logging which meant that it
was implicitly loading log4j. This caused some noisy logs during
shutdown when the previously unused cache was initialized just to be
cleaned up.
This commit also upgrades caffeine and moving forward we can always
upgrade caffeine (and potentially shade it) without any conflict with
the scalacache version.
Upon successful registration with a FileTreeRepository, an Observable is
returned by the FileTreeRepository that can be used to observer the
specific globs that were registered. The FileTreeRepository also has a
global Observable that can be used to monitor _all_ events. In order to
implement this feature, internally the FileTreeRepository needs to hold
a reference to the registered Observable so that it forwards relevant
file change events. If we do not close the Observable, it leaks memory
inside of FileTreeRepository. There were a number of places within sbt
where we registered globs and did nothing with the returned Observable.
It was thus straightforward to fix the leak by just closing the returned
Observables.
This came up because I was looking at a heap dump of
https://github.com/jtjeferreira/sbt-multi-module-sample after running
1000 no-op compiles and noticed that the FileTreeRepository.observables
were taking up 75MB out of a total heap of about 300MB.
As a side note, it would be nice if sbt had a warning for unused return
values when a statement is not the last in a block. It's possible that
these leaks wouldn't have happened if we were forced to handle the
returned Observables.
Ref https://github.com/sbt/zinc/pull/744
This implements `ThisBuild / usePipelining`, which configures subproject pipelining available from Zinc 1.4.0.
The basic idea is to start subproject compilation as soon as pickle JARs (early output) becomes available. This is in part enabled by Scala compiler's new flags `-Ypickle-java` and `-Ypickle-write`.
The other part of magic is the use of `Def.promise`:
```
earlyOutputPing := Def.promise[Boolean],
```
This notifies `compileEarly` task, which to the rest of the tasks would look like a normal task but in fact it is promise-blocked. In other words, without calling full `compile` task together, `compileEarly` will never return, forever waiting for the `earlyOutputPing`.
It can easily take 2ms or more to parse a command depending on state's
combined parser. There are some commands that sbt requires to work that
we can handle in microseconds instead of milliseconds by special casing
them.
After this change, I saw the performance of
https://github.com/eatkins/scala-build-watch-performance improve by
a consistent 4-5ms in the 3 source file example which was a drop from
120ms to 115ms. While not necessarily earth shattering, this difference
could theoretically be much worse in other projects that have a lot of
plugins and custom tasks/commands. I think it's worth the modest
maintenance cost.
Ref https://github.com/sbt/sbt/issues/5710
Ref https://github.com/sbt/librarymanagement/pull/339
This adds `versionScheme` setting. When set, it is included into POM, and gets picked up on the other side as an extra attribute of ModuleID. That information in turn is used to inform the eviction warning.
This should reduce the false positives associated with SemVer'ed libraries showing up in the eviction warning.
The 1.4.0 implementation of watch uses a concurrent hash map to maintain
the global watch state which manages the state for an arbitrary number
of clients. Using a mutable map is not idiomatic sbt and I found it
difficult to reason about when the map was updated. This commit reworks
the feature so that the global state is instead stored in an immutable
map that is only modified during the internal watch commands, which is
easier to reason about.
The EventsTest changes kept appearing. I'm not sure why scalafmt check
was allowing it before. My vim status bar warns me about trailing spaces
and I noticed the two in Keys.scala and removed them.
In eb688c9ecd, we started buffering output
to the remote client to reduce flickering. This was causing problems
with the output for the thin client in batch mode. With the delay, it
was possible for the client to exit before all of its output had been
displayed.
Bonus: only display aggregation error message if terminal has success
enabled (the thin client displays its own timing message so the message
in aggregation ended up being a duplicate).
It is expensive to compute the the hash of every jar on the classpath so
we can try to avoid that by using the timeWrappedStamper which only
computes the hash if the last modified time has changed.
Using the managedCached introduced an unintended performance regression
because it ensured that we always computed the hash of each jar on the
dependency classpath. The backing ReadStamps only computes the stamp if
the timestamp of the jar has changed.
I noticed that if lintUnusedKeysOnLoad := true is set, it emits a lint
warning.
As a side note, Project linting takes about 300-400ms in the sbt project
so we might want to consider disabling it by default in batch mode at
least.
The sbt project load made a number of relatively inefficient
transformations of scala collecitons. I went through and found the slow
parts during project loading and made my best attempt at fixing them.
The most significant changes I made were in places using IMap. An IMap
is more or less a wrapper around an immutable Map. It can be much faster
to construct an IMap by creating a java mutable hashmap, wrapping it a
scala Map that delegates to the underlying java hashmap (with a copy on
write if the map is modified) and constructing the IMap from the wrapped
map. It was also in many cases to parallelize some transformations
wherever the order didn't matter.
After applying all of these changes, I found that loading the sbt
project took generally between 8.5 and 9 seconds on my laptop. With
1.3.13, it hovered around 11.5 seconds. I saw a similar speedup in zinc.
The biggest specific improvement was that generating the compiled map
dropped from between 3.5-4 seconds to pretty consistently being around
1.5 seconds.
This reverts commit b1dcf031a5.
I found that b1dcf031a5 had some
unintended consequences that seemed to mess up the prompt state. The
real problem that it was trying to address was that the prompt was being
interleaved with log messages in some scenarios. There was a different
way to fix that in ProgressState that was both simpler and more
reliable.
The jline2 history file format is incompatible with jline3 and jline3
prints a very noisy warning if it detects such a file. History will also
not work with jline3 until you remove or reformat the old file.
I noticed that when reloading the build, that certain errors are logged
by sbt to System.err. These were not shown to a thin client because we
weren't forwarding System.err. This change remedies that.
System.err is handled more simply than System.out. We do not put
System.err through the progress state because generally System.err is
tends to be unbuffered. I had hesitated to add System.err to the
Terminal interface at all to give users an escape hatch but I couldn't
get project loading to work well with the thin client without it.
In db4878c786, TaskProgress became an
object which mostly made things easier to reason about. The one problem
was that it started leaking tasks with every run because the timings map
would accumulate tasks that weren't cleared. To fix this, we can clear
the timings and activeTasksMap in the TaskProgress object in the
afterAllCompleted callback. Some extra null checks needed to be added
since it's possible for the maps to not contain a previously added key
after reset has been called.
This fixes the nio/external-hooks test and also restores the performance
of the benchmarks for the latest sbt version in
https://github.com/eatkins/scala-build-watch-performance which had
regressed when the custom ExternalHooks were disabled in
7c4b01d9f7.
The main change is that it changes the ReadStamps object that is passed
into the compiler options to one that uses the unmanagedFileStampCache
and managedFileStampCache for source files and falls back to the default
stamper otherwise. This improves the performance quite significantly
since we only hash the files once. It also makes it so that the analysis
file will contain the source file stamps of the files when compilation
began, rather than when compilation ended. That is what
nio/external-hooks was testing. In the real world what could happen was
that one modified a source file during compilation but then no
incremental re-compilation would occur because after the initial
compilation completed, zinc wrote the stamp of the modified source file
in the analysis file even though it may have actually compiled a
different version of the source file.
I noticed some RejectedExecutionExceptions in travis failures of
ClientTest. This could happen if we try to write output to the
network channel after it has been closed. To avoid this problem, we can
catch RejectedExecutionExceptions and do an immediate flush if the
executor has been shutdown.
In order for sbt to function well, it needs the test interface, jansi
and forked jline jars provided by a classloader that is parent to all
other sbt classloaders. To do this for just the test interface jar, I
just checked if the top loader in the app configuration had the correct
name. Now that there are three jars, this is more complicated so I
updated the launcher to create a top loader with the method getEarlyJars
implemented and returning the three needed jars. This is a much more
scalable design.
If sbt is entered with a configuration that does not have a top loader
with the getEarlyJars method defined, then we just fall back on
constructing the default layered classlaoder from the configuration
classpath.
The motivation for this change is that I discovered that sbt immediately
crashed when I tried to run a non-snapshot version. After this change, I
verified that both snapshot and non-snapshot versions of the latest sbt
code could load with both an obsolete and up-to-date launcher.
The unprompt method will actually kill the ui thread if its running. If
we don't to this, we can get into a weird state where after watch is
exited by <enter>, the ask user thread spins up but before it can print
the prompt, the terminal prompt is changed to Running, which has an
empty prompt. The end result is that jline3 never displays the prompt
even though the line reader is active and reading bytes. When the user
typed <enter> a prompt would appear. They also could input a command and
it would run but it wasn't obvious what would happen since the prompt
was missing.
The build source check is evaluated at times when we can't be completely
sure that global logger is pointing at the terminal that initiated the
reload (which may be a passive watch client). To work around this, we
can inspect the exec to determine which terminal initiated the check and
write any output directly to that terminal.
This commit upgrades sbt to using jline3. The advantage to jline3 is
that it has a significantly better tab completion engine that is more
similar to what you get from zsh or fish.
The diff is bigger than I'd hoped because there are a number of
behaviors that are different in jline3 vs jline2 in how the library
consumes input streams and implements various features. I also was
unable to remove jline2 because we need it for older versions of the
scala console to work correctly with the thin client. As a result, the
changes are largely additive.
A good amount of this commit was in adding more protocol so that the
remote client can forward its jline3 terminal information to the server.
There were a number of minor changes that I made that either fixed
outstanding ui bugs from #5620 or regressions due to differences between
jline3 and jline2.
The number one thing that caused problems is that the jline3 LineReader
insists on using a NonBlockingInputStream. The implementation ofo
NonBlockingInputStream seems buggy. Moreover, sbt internally uses a
non blocking input stream for system in so jline is adding non blocking
to an already non blocking stream, which is frustrating.
A long term solution might be to consider insourcing LineReader.java
from jline3 and just adapting it to use an sbt terminal rather than
fighting with the jline3 api. This would also have the advantage of not
conflicting with other versions of jline3. Even if we don't, we may want to
shade jline3 if that is possible.
It is possible for sbt to get into a weird state when in a continuous
build when the auto reload feature is on and a source file and a build
file are changed in a small window of time. If sbt detects the source
file first, it will start running the command but then it will
autoreload when it runs the command because of the build file change.
This causes the watch to get into a broken state because it is necessary
to completely restart the watch after sbt exits.
To fix this, we can aggregate the detected events in a 100ms window. The
idea is to handle bursts of file events so we poll in 5ms increments and
as soon as no events are detected, we trigger a build.
Remote clients sometimes flicker when updating progress. This is especially
noticeable when there are two clients and one of them is running a command,
the other will tend to have some visible flickering and character ghosting.
As an experiment, I buffered calls to flush in the NetworkChannel output
stream and the artifacts went away.
Running a `~` command in a local build off the latest develop branch
will cause the build to reload even if the build sources were only
touched and not actually modified.
In the situation where sbt was started in server mode and a client is
running a `~` command and a project reload is triggered by a change to
a build source, the console terminal looks like
sbt:foo>
[info] received remote command: ~compile
sbt:foo>
[info] welcome to sbt 1.4.0-SNAPSHOT (Azul Systems, Inc. Java 1.8.0_252)
sbt:foo>
[info] loading global plugins from ~/.sbt/1.0/plugins
sbt:foo>
[info] loading settings for project foo-build from metals.sbt ...
sbt:foo>
[info] loading project definition from
~/foo/project
sbt:foo>
[info] loading settings for project root from build.sbt ...
sbt:foo>
[info] loading settings for project macros from build.sbt ...
sbt:foo>
[info] loading settings for project main from build.sbt ...
sbt:foo>
[info] set current project to foo (in build file:~/foo)
sbt:foo>
This change fixes that by unprompting all channels during project
loading and reprompting them when it completes.
Linting unused keys was adding a significant overhead to sbt project
loading because Def.compiled is so slow. It was around 4 seconds in the
sbt project on my computer.
The continuous command recompiles the setting graph into a CompiledMap
data structure so that it can determine which files it needs to
transitively monitor during watch. Generating the CompiledMap can be
very slow for large projects (5 seconds or so on my computer in the sbt
project) and this startup cost is paid every time the user enters a
watch with `~`. To avoid this, we can cache the compile map that is
generated during the initial settings evaluation.
The only real drawback I can see is that the compiled map is guaranteed
to remain in memory so long as the BuildStructure instance that holds it
is alive. Given the performance benefit, this seems like a worthwhile
tradeoff.
There have been occasional failures on appveyor where an
AccessDeniedException was thrown at this point. AccessDeniedExceptions
thrown during scripted tests can often by resolved with a Retry.
Reboot is a bit tricky for the remote client because the sbt server is
actually shut down during reboot. When sbt shuts down the client, it can
notify the client that the reason is a reboot. The client can then
connect to the recently introduced boot control socket to display the
reboot output and supply input in case the build fails to load. Once the
server has brought back up the server, the client can reconnect. When
the client session is interactive, we're done once we reconnect. When
it's a batch session, the client needs to resend the remaing commands
that have submitted that it hasn't yet run.
Shutdown was being handled as a special case in CommandExchange. This
promotes it to a full fledged command. Also replace instance of
hard-coded strings with constants.
On windows, it is sometimes possible to leak an sbt process if two
processes are started simultaneously by a remote client at the same
time. When this happens, the second process is unable to create a
server because of the first process and it also has no io streams
because the the client detaches its streams. We can detect this
in the shell command and prevent the process from persisting as a
zombie.
When a remote client sent the command `shutdown` through the shell, the
client would log an error and exit with a nonzero exit code because
before shutting down, the server would notify the client that it was
disconnecting it due to shutdown. In this scenario, we actually do not
want the client to log an error since they initiated the shutdown, so
before doing the full shutdown, we shutdown the client that inititated
the shutdown with the flag that tells the client not to log the shutdown
or return a nonzero exit code.
One issue with the remote client approach is that it is possible for
multiple clients to start multiple servers concurrently. I encountered
this in testing where in one tmux pane I'd start an sbt server and in
another I might run sbtc before the server had finished loading. This
can actually cause java processes to leak because the second process is
unable to start a server but it doesn't necessarily die after the client
that spawned it exits. This commit prevents this scenario by creating a
server socket before it loads the build and closes once the build is
complete. The client can then receive output bytes and forward input to
the booting server.
The socket that is created during boot is always a local socket, either
a UnixDomainServerSocket or a Win32NamedPipeServerSocket. At the moment,
I don't see any reason to support TCP. This socket also has no impact at
all on the normal sbt server that is started up after the project has
loaded.
The socket is hardcoded to be located at the relative path
project/target/$SOCK_NAME or the named pipe $SOCK_NAME where SOCK_NAME
is a farm hash of the absolute path of the project base directory. There
is no portfile json since there is no need since we don't support TCP.
After the socket is created it listens for clients to whom it relays
input to the console's input stream and relays the process output back
to the client. See the javadoc in BootServerSocket.java for further
details.
The process for forking the server is also a bit more complicated after
this change because the client will read the process output and error
streams until the socket is created and thereafter will only read output
from the socket, not the process.
This commit reworks TaskProgress so that it is a singleton object. By
using a singleton, we ensure that there is at most one progress thread
running at a time. With multiple threads, there can be flickering in the
progress reports.
This fixes https://github.com/sbt/sbt/issues/5547. There also was a bug
that the reference to the progress thread was not reset when the thread
itself exited. As a result, it was possible for progress reporting to
stop while tasks were still running. This seemed to primarily happen in
multi-project builds. It should be fixed by this change.
Ethan Atkins
eugene yokota
Adrien Piquerez