Skip to content

Commit

Permalink
Multi-line status prompt ticker (#3577)
Browse files Browse the repository at this point in the history
fixes #3546

This PR introduces a new `MultilinePromptLogger` that allows the status
of multiple in-progress threads to be reported and viewed during a Mill
evaluation:



https://github.com/user-attachments/assets/32d2a7e4-6db9-4dcc-81ed-a775e307d1c2


`MultilinePromptLogger` is basically a minimal translation of the
current `ticker` API to work with a multi-line prompt. There's probably
a lot of other interesting improvements we can make now that we are
multi-line, but this is a decent start that lets people know what their
parallel build is doing. The UI works correctly both during scrolling
and not-scrolling, and uses the same minimal ANSI codes that the
existing ticker uses, so hopefully we won't hit any more edge cases that
we aren't already hitting today

From an implementation perspective, `MultilinePromptLogger` is mostly a
drop in replacement for the old `PrintLogger`.

User-facing notes:

1. It prints multiple `ticker` lines at vertically at the bottom of the
terminal, and has the logs printed above

2. It requires that you use `.endTicker()` after every `.ticker("...")`
call, so we can know where we should clear the ticker status (previously
they always just got overriden by the next `.ticker` call)

3. We need to introduce a `withPaused{...}` block so that when you're
running REPLs and stuff the prompt is not shown

4. We only can support logs which end with newlines. This means things
like interactive progress bars and other ANSI magic won't work in the
scrollback. This is a limitation of the current implementation that is
hard to fix without going for more advanced techniques, but should be
enough for the vast majority of logs and is a similar limitation as a
lot of other tools.

5. Console dimensions are propagated from the mill client to the mill
server via a `terminfo` file specific to each server. This is used to
size the prompt so it takes up the whole width and about 1/3 of the
height. This happens every 100ms, so there may be some delay, but at
least it means the prompt will right-size itself on next render. The
regular cadence also means it shouldn't be a performance bottleneck

6. For non-interactive runs which lack a terminal, prompt WxH defaults
to 120x50, and we tweak the rendering: we no longer render blank lines
to preserve prompt height stability, we render less frequently and only
if the statuses change, and we add a footer so the bottom of the prompt
is clearly marked.

7. `--ticker` now defaults to `true` even for non-interactive runs,
since the periodic display of prompt (I set to 60s intervals, if prompt
changes) is useful e.g. for debugging stuck background jobs or CI runs.

Implementation Notes

1. The logger needs to extend `AutoCloseable`, since it uses a
background thread to keep the prompt UI up to date. This renders every
100ms (arbitrary) to try and balance between prompt readability and
latency. We have additional delays built in to hiding a status line and
then finally removing it, to try and preserve the height so it doesn't
bounce up and down too much as the set of running tasks changes

2. We re-use the `ProxyStream` classes to combine the stderr/stdout
before re-splitting them. This allows us to perform some buffering,
while simultaneously maintaining ordering of writes to each stream,
while also allowing us to detect quiescence so we can only bother
writing out a prompt when everything else is done and it won't get
immediately over-written. `ProxyStream.Pumper` needed some tweaks to add
hooks and remove client-specific assumptions

3. I experimented with having the ticker logic live in the Mill client
rather than server, which would make more sense, but we need the server
to have the ability to enable/disable the ticker logic to run `console`
and similar interactive tasks, and so it has to live in the server


The old ticker remains available at `--disable-prompt`. Further
improvements can come after 0.12.0.
  • Loading branch information
lihaoyi authored Sep 26, 2024
1 parent fd46d8e commit 1780314
Show file tree
Hide file tree
Showing 47 changed files with 2,627 additions and 464 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/run-mill-action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ jobs:
if: inputs.millargs != '' && startsWith(inputs.os, 'windows')

- name: Run Mill (on Windows) Worker Cleanup
run: 'taskkill -f -im java* && rm -rf out/mill-worker-*'
run: 'taskkill -f -im java* && rm -rf out/mill-server/*'
if: inputs.millargs != '' && startsWith(inputs.os, 'windows')
shell: bash
continue-on-error: true
Expand Down
1 change: 1 addition & 0 deletions bsp/src/mill/bsp/BspContext.scala
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ private[mill] class BspContext(
override def info(s: String): Unit = streams.err.println(s)
override def error(s: String): Unit = streams.err.println(s)
override def ticker(s: String): Unit = streams.err.println(s)
override def ticker(key: String, s: String): Unit = streams.err.println(s)
override def debug(s: String): Unit = streams.err.println(s)

override def debugEnabled: Boolean = true
Expand Down
2 changes: 1 addition & 1 deletion build.mill
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import mill.define.Cross
import $meta._
import $file.ci.shared
import $file.ci.upload

//import $packages._
object Settings {
val pomOrg = "com.lihaoyi"
val githubOrg = "com-lihaoyi"
Expand Down
6 changes: 3 additions & 3 deletions docs/modules/ROOT/pages/Out_Dir.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ Mill puts all its output in the top-level `out/` folder.
== Structure of the `out/` Directory

The `out/` folder contains all the generated files & metadata for your build.
It holds some files needed to manage Mill's longer running server instances (`out/mill-worker-*`) as well as a directory and file structure resembling the project's module structure.
It holds some files needed to manage Mill's longer running server instances (`out/mill-server/*`) as well as a directory and file structure resembling the project's module structure.

.Example of the `out/` directory after running `mill main.compile`
[source,text]
Expand Down Expand Up @@ -47,7 +47,7 @@ out/
│ ├── unmanagedClasspath.json
│ └── upstreamCompileOutput.json
├── mill-profile.json
└── mill-worker-VpZubuAK6LQHHN+3ojh1LsTZqWY=-1/
└── mill-server/VpZubuAK6LQHHN+3ojh1LsTZqWY=-1/
----

<1> The `main` directory contains all files associated with target and submodules of the `main` module.
Expand Down Expand Up @@ -109,5 +109,5 @@ This is very useful if Mill is being unexpectedly slow, and you want to find out
`mill-chrome-profile.json`::
This file is only written if you run Mill in parallel mode, e.g. `mill --jobs 4`. This file can be opened in Google Chrome with the built-in `tracing:` protocol even while Mill is still running, so you get a nice chart of what's going on in parallel.

`mill-worker-*/`::
`mill-server/*`::
Each Mill server instance needs to keep some temporary files in one of these directories. Deleting it will also terminate the associated server instance, if it is still running.
4 changes: 2 additions & 2 deletions example/depth/sandbox/1-task/build.mill
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,14 @@ def osProcTask = Task {
//
// Lastly, there is the possibily of calling `os.pwd` outside of a task. When outside of
// a task there is no `.dest/` folder associated, so instead Mill will redirect `os.pwd`
// towards an empty `sandbox/` folder in `out/mill-worker.../`:
// towards an empty `sandbox/` folder in `out/mill-server/...`:

val externalPwd = os.pwd
def externalPwdTask = Task { println(externalPwd.toString) }

/** Usage
> ./mill externalPwdTask
.../out/mill-worker-.../sandbox/sandbox
.../out/mill-server/.../sandbox
*/


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ object MissingBuildFileTests extends UtestIntegrationTestSuite {
test - integrationTest { tester =>
val res = tester.eval(("resolve", "_"))
assert(!res.isSuccess)
val s"build.mill file not found in $msg. Are you in a Mill project folder?" = res.err
val s"${prefix}build.mill file not found in $msg. Are you in a Mill project folder?" = res.err
}
}
}
16 changes: 16 additions & 0 deletions main/api/src/mill/api/Logger.scala
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,14 @@ trait Logger {
def info(s: String): Unit
def error(s: String): Unit
def ticker(s: String): Unit
def ticker(key: String, s: String): Unit = ticker(s)
private[mill] def reportPrefix(s: String): Unit = ()
private[mill] def promptLine(key: String, identSuffix: String, message: String): Unit =
ticker(s"$key $message")
private[mill] def globalTicker(s: String): Unit = ()
private[mill] def clearAllTickers(): Unit = ()
private[mill] def endTicker(key: String): Unit = ()

def debug(s: String): Unit

/**
Expand All @@ -53,4 +61,12 @@ trait Logger {
def debugEnabled: Boolean = false

def close(): Unit = ()

/**
* Used to disable the terminal UI prompt without a certain block of code so you
* can run stuff like REPLs or other output-sensitive code in a clean terminal
*/
def withPromptPaused[T](t: => T) = t

def enableTicker: Boolean = false
}
120 changes: 91 additions & 29 deletions main/api/src/mill/api/SystemStreams.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package mill.api
import java.io.{InputStream, OutputStream, PrintStream}
import mill.main.client.InputPumper

import scala.util.DynamicVariable

/**
* Represents a set of streams that look similar to those provided by the
* operating system. These may internally be proxied/redirected/processed, but
Expand All @@ -28,11 +30,12 @@ object SystemStreams {
* stdout/stderr/stdin.
*/
def isOriginal(): Boolean = {
(System.out eq original.out) &&
(System.err eq original.err) &&
(System.in eq original.in) &&
(Console.out eq original.out) &&
(Console.err eq original.err)
(Console.out eq original.out) && (Console.err eq original.err)
// We do not check System.* for equality because they are always overridden by
// `ThreadLocalStreams`
// (System.out eq original.out) &&
// (System.err eq original.err) &&
// (System.in eq original.in) &&

// We do not check `Console.in` for equality, because `Console.withIn` always wraps
// `Console.in` in a `new BufferedReader` each time, and so it is impossible to check
Expand Down Expand Up @@ -62,30 +65,24 @@ object SystemStreams {
Some(new InputPumper(() => processOut.wrapped, () => dest, false, () => true))
}
def withStreams[T](systemStreams: SystemStreams)(t: => T): T = {
val in = System.in
val out = System.out
val err = System.err
try {
// If we are setting a stream back to its original value, make sure we reset
// `os.Inherit` to `os.InheritRaw` for that stream. This direct inheritance
// ensures that interactive applications involving console IO work, as the
// presence of a `PumpedProcess` would cause most interactive CLIs (e.g.
// scala console, REPL, etc.) to misbehave
val inheritIn =
if (systemStreams.in eq original.in) os.InheritRaw
else new PumpedProcessInput

val inheritOut =
if (systemStreams.out eq original.out) os.InheritRaw
else new PumpedProcessOutput(systemStreams.out)

val inheritErr =
if (systemStreams.err eq original.err) os.InheritRaw
else new PumpedProcessOutput(systemStreams.err)

System.setIn(systemStreams.in)
System.setOut(systemStreams.out)
System.setErr(systemStreams.err)
// If we are setting a stream back to its original value, make sure we reset
// `os.Inherit` to `os.InheritRaw` for that stream. This direct inheritance
// ensures that interactive applications involving console IO work, as the
// presence of a `PumpedProcess` would cause most interactive CLIs (e.g.
// scala console, REPL, etc.) to misbehave
val inheritIn =
if (systemStreams.in eq original.in) os.InheritRaw
else new PumpedProcessInput

val inheritOut =
if (systemStreams.out eq original.out) os.InheritRaw
else new PumpedProcessOutput(systemStreams.out)

val inheritErr =
if (systemStreams.err eq original.err) os.InheritRaw
else new PumpedProcessOutput(systemStreams.err)

ThreadLocalStreams.current.withValue(systemStreams) {
Console.withIn(systemStreams.in) {
Console.withOut(systemStreams.out) {
Console.withErr(systemStreams.err) {
Expand All @@ -99,10 +96,75 @@ object SystemStreams {
}
}
}
}
}

/**
* Manages the global override of `System.{in,out,err}`. Overrides of those streams are
* global, so we cannot just override them per-use-site in a multithreaded environment
* because different threads may interleave and stomp over each other's over-writes.
* Instead, we over-write them globally with a set of streams that does nothing but
* forward to the per-thread [[ThreadLocalStreams.current]] streams, allowing callers
* to each reach their own thread-local streams without clashing across multiple threads
*/
def withTopLevelSystemStreamProxy[T](t: => T): T = {
val in = System.in
val out = System.out
val err = System.err

try {
setTopLevelSystemStreamProxy()
t
} finally {
System.setErr(err)
System.setOut(out)
System.setIn(in)
}
}
def setTopLevelSystemStreamProxy(): Unit = {
// Make sure to initialize `Console` to cache references to the original
// `System.{in,out,err}` streams before we redirect them
Console.out
Console.err
Console.in
System.setIn(ThreadLocalStreams.In)
System.setOut(ThreadLocalStreams.Out)
System.setErr(ThreadLocalStreams.Err)
}

private[mill] object ThreadLocalStreams {
val current = new DynamicVariable(original)

object Out extends PrintStream(new ProxyOutputStream { def delegate() = current.value.out })
object Err extends PrintStream(new ProxyOutputStream { def delegate() = current.value.err })
object In extends ProxyInputStream { def delegate() = current.value.in }

abstract class ProxyOutputStream extends OutputStream {
def delegate(): OutputStream
override def write(b: Array[Byte], off: Int, len: Int): Unit = delegate().write(b, off, len)
override def write(b: Array[Byte]): Unit = delegate().write(b)
def write(b: Int): Unit = delegate().write(b)
override def flush(): Unit = delegate().flush()
override def close(): Unit = delegate().close()
}
abstract class ProxyInputStream extends InputStream {
def delegate(): InputStream
override def read(): Int = delegate().read()
override def read(b: Array[Byte], off: Int, len: Int): Int = delegate().read(b, off, len)
override def read(b: Array[Byte]): Int = delegate().read(b)
override def readNBytes(b: Array[Byte], off: Int, len: Int): Int =
delegate().readNBytes(b, off, len)
override def readNBytes(len: Int): Array[Byte] = delegate().readNBytes(len)
override def readAllBytes(): Array[Byte] = delegate().readAllBytes()
override def mark(readlimit: Int): Unit = delegate().mark(readlimit)
override def markSupported(): Boolean = delegate().markSupported()
override def available(): Int = delegate().available()
override def reset(): Unit = delegate().reset()
override def skip(n: Long): Long = delegate().skip(n)
// Not present in some versions of Java
// override def skipNBytes(n: Long): Unit = delegate().skipNBytes(n)
override def close(): Unit = delegate().close()
override def transferTo(out: OutputStream): Long = delegate().transferTo(out)
}
}
}
20 changes: 20 additions & 0 deletions main/client/src/mill/main/client/DebugLog.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package mill.main.client;
import java.io.IOException;
import java.nio.file.*;

/**
* Used to add `println`s in scenarios where you can't figure out where on earth
* your stdout/stderr/logs are going and so we just dump them in a file in your
* home folder so you can find them
*/
public class DebugLog{
synchronized public static void println(String s){
Path path = Paths.get(System.getProperty("user.home"), "mill-debug-log.txt");
try {
if (!Files.exists(path)) Files.createFile(path);
Files.writeString(path, s + "\n", StandardOpenOption.APPEND);
}catch (IOException e){
throw new RuntimeException(e);
}
}
}
3 changes: 2 additions & 1 deletion main/client/src/mill/main/client/OutFiles.java
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,12 @@ public class OutFiles {
* Subfolder of `out/` that contains the machinery necessary for a single Mill background
* server: metadata files, pipes, logs, etc.
*/
final public static String millWorker = "mill-worker-";
final public static String millServer = "mill-server";

/**
* Subfolder of `out/` used to contain the Mill subprocess when run in no-server mode
*/
final public static String millNoServer = "mill-no-server";


}
19 changes: 13 additions & 6 deletions main/client/src/mill/main/client/ProxyStream.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.util.function.BiConsumer;
import java.util.function.Consumer;

/**
* Logic to capture a pair of streams (typically stdout and stderr), combining
Expand Down Expand Up @@ -106,11 +108,16 @@ public Pumper(InputStream src, OutputStream destOut, OutputStream destErr){
this.destErr = destErr;
}

public void preRead(InputStream src){}

public void preWrite(){}

public void run() {

byte[] buffer = new byte[1024];
while (true) {
try {
this.preRead(src);
int header = src.read();
// -1 means socket was closed, 0 means a ProxyStream.END was sent. Note
// that only header values > 0 represent actual data to read:
Expand All @@ -124,6 +131,7 @@ public void run() {
int offset = 0;
int delta = -1;
while (offset < quantity) {
this.preRead(src);
delta = src.read(buffer, offset, quantity - offset);
if (delta == -1) {
break;
Expand All @@ -133,26 +141,25 @@ public void run() {
}

if (delta != -1) {
this.preWrite();
switch(stream){
case ProxyStream.OUT: destOut.write(buffer, 0, offset); break;
case ProxyStream.ERR: destErr.write(buffer, 0, offset); break;
}

flush();
}
}
} catch (org.newsclub.net.unix.ConnectionResetSocketException e) {
// This happens when you run mill shutdown and the server exits gracefully
break;
} catch (IOException e) {
e.printStackTrace();
System.exit(1);
// This happens when the upstream pipe was closed
break;
}
}

try {
destOut.close();
destErr.close();
destOut.flush();
destErr.flush();
} catch(IOException e) {}
}

Expand Down
11 changes: 8 additions & 3 deletions main/client/src/mill/main/client/ServerFiles.java
Original file line number Diff line number Diff line change
@@ -1,22 +1,22 @@
package mill.main.client;

/**
* Central place containing all the files that live inside the `out/mill-worker-*` folder
* Central place containing all the files that live inside the `out/mill-server-*` folder
* and documentation about what they do
*/
public class ServerFiles {
final public static String serverId = "serverId";
final public static String sandbox = "sandbox";

/**
* Ensures only a single client is manipulating each mill-worker folder at
* Ensures only a single client is manipulating each mill-server folder at
* a time, either spawning the server or submitting a command. Also used by
* the server to detect when a client disconnects, so it can terminate execution
*/
final public static String clientLock = "clientLock";

/**
* Lock file ensuring a single server is running in a particular mill-worker
* Lock file ensuring a single server is running in a particular mill-server
* folder. If multiple servers are spawned in the same folder, only one takes
* the lock and the others fail to do so and terminate immediately.
*/
Expand Down Expand Up @@ -67,4 +67,9 @@ public static String pipe(String base) {
* Where the server's stderr is piped to
*/
final public static String stderr = "stderr";

/**
* Terminal information that we need to propagate from client to server
*/
final public static String terminfo = "terminfo";
}
Loading

0 comments on commit 1780314

Please sign in to comment.