Skip to content

Commit

Permalink
Optimize startup time using TCP sockets instead of junixsocket and `t…
Browse files Browse the repository at this point in the history
…put` instead of jline (#4009)

The `AFUNIXServerSocket` library we are using seems to cause an extra
~500ms latency launching the Mill client, and also causes the Graal
native-image to crash during generation.

* Using TCP sockets seems to cut the launch overhead down from ~1000ms
to ~500ms, and opens up the possibility of using native-image to cut it
further.

* Using localhost TCP sockets seems secure as far as I can tell
https://security.stackexchange.com/questions/108544/once-established-are-sockets-on-localhost-secure

* The JVM also ships with `UnixDomainSocketAddress` starting from JDK 17
that we can consider using, either by requiring JDK17 or by
conditionally using it based on JDK version. But for now I want to keep
supporting JDK11 and don't want to split the code paths, so we'll hold
off on this for now

`jline.terminal.Terminal.getSize` also adds a few hundred milliseconds,
so we instead use the same `tput` command that Ammonite uses which takes
<10ms.

* On windows where `tput` isn't support, it falls back to 100 cols,
which I reduced it down from 120 to hopefully avoid line wrapping in
most terminals while still providing a decent experience

Together these two changes cuts down the time taken for a hot `time
./mill version` on the Mill repo from ~1.05s to ~0.24s
  • Loading branch information
lihaoyi authored Nov 23, 2024
1 parent a900a9b commit b1f6be0
Show file tree
Hide file tree
Showing 10 changed files with 53 additions and 49 deletions.
8 changes: 4 additions & 4 deletions integration/feature/full-run-logs/src/FullRunLogsTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,17 @@ object FullRunLogsTests extends UtestIntegrationTestSuite {
assert("\\[\\d+\\] <h1>hello</h1>".r.matches(res.out))

val expectedErrorRegex =
s"""==================================================== run --text hello ================================================
|======================================================================================================================
s"""========================================== run --text hello ======================================
|==================================================================================================
|[build.mill-<digits>/<digits>] compile
|[build.mill-<digits>] [info] compiling 1 Scala source to ${tester.workspacePath}/out/mill-build/compile.dest/classes ...
|[build.mill-<digits>] [info] done compiling
|[<digits>/<digits>] compile
|[<digits>] [info] compiling 1 Java source to ${tester.workspacePath}/out/compile.dest/classes ...
|[<digits>] [info] done compiling
|[<digits>/<digits>] run
|[<digits>/<digits>] ============================================ run --text hello ============================================<digits>s
|======================================================================================================================"""
|[<digits>/<digits>] ================================== run --text hello ==================================<digits>s
|=================================================================================================="""
.stripMargin
.replaceAll("(\r\n)|\r", "\n")
.replace('\\', '/')
Expand Down
6 changes: 4 additions & 2 deletions main/api/src/mill/api/ClassLoader.scala
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import java.net.{URL, URLClassLoader}
import java.nio.file.{FileAlreadyExistsException, FileSystemException}

import mill.java9rtexport.Export
import scala.util.{Properties, Try}
import scala.util.Properties

/**
* Utilities for creating classloaders for running compiled Java/Scala code in
Expand Down Expand Up @@ -90,7 +90,9 @@ object ClassLoader {
)
retry {
try os.copy(os.Path(Export.rt()), java90rtJar, createFolders = true)
catch { case e: FileAlreadyExistsException => /* someone else already did this */}
catch {
case e: FileAlreadyExistsException => /* someone else already did this */
}
}
}
urls :+ java90rtJar.toIO.toURI().toURL()
Expand Down
3 changes: 0 additions & 3 deletions main/client/src/mill/main/client/ProxyStream.java
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,6 @@ public void run() {
}
}
}
} catch (org.newsclub.net.unix.ConnectionResetSocketException e) {
// This happens when you run mill shutdown and the server exits gracefully
break;
} catch (IOException e) {
// This happens when the upstream pipe was closed
break;
Expand Down
5 changes: 5 additions & 0 deletions main/client/src/mill/main/client/ServerFiles.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ public class ServerFiles {
*/
public static final String processLock = "processLock";

/**
* The port used to connect between server and client
*/
public static final String socketPort = "socketPort";

/**
* The pipe by which the client snd server exchange IO
*
Expand Down
9 changes: 2 additions & 7 deletions main/client/src/mill/main/client/ServerLauncher.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import static mill.main.client.OutFiles.*;

import java.io.File;
import java.io.InputStream;
import java.io.OutputStream;
import java.io.PrintStream;
Expand All @@ -13,8 +12,6 @@
import java.util.Map;
import mill.main.client.lock.Locks;
import mill.main.client.lock.TryLocked;
import org.newsclub.net.unix.AFUNIXSocket;
import org.newsclub.net.unix.AFUNIXSocketAddress;

/**
* Client side code that interacts with `Server.scala` in order to launch a generic
Expand Down Expand Up @@ -133,15 +130,13 @@ int run(Path serverDir, boolean setJnaNoSys, Locks locks) throws Exception {

while (locks.processLock.probe()) Thread.sleep(3);

String socketName = ServerFiles.pipe(serverDir.toString());
AFUNIXSocketAddress addr = AFUNIXSocketAddress.of(new File(socketName));

long retryStart = System.currentTimeMillis();
Socket ioSocket = null;
Throwable socketThrowable = null;
while (ioSocket == null && System.currentTimeMillis() - retryStart < serverInitWaitMillis) {
try {
ioSocket = AFUNIXSocket.connectTo(addr);
int port = Integer.parseInt(Files.readString(serverDir.resolve(ServerFiles.socketPort)));
ioSocket = new java.net.Socket("127.0.0.1", port);
} catch (Throwable e) {
socketThrowable = e;
Thread.sleep(10);
Expand Down
1 change: 0 additions & 1 deletion main/package.mill
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,6 @@ object `package` extends RootModule with build.MillStableScalaModule with BuildI
object client extends build.MillPublishJavaModule with BuildInfo {
def buildInfoPackageName = "mill.main.client"
def buildInfoMembers = Seq(BuildInfo.Value("millVersion", build.millVersion(), "Mill version."))
def ivyDeps = Agg(build.Deps.junixsocket, build.Deps.jline)

object test extends JavaModuleTests with TestModule.Junit4 {
def ivyDeps = Agg(build.Deps.junitInterface, build.Deps.commonsIo)
Expand Down
18 changes: 3 additions & 15 deletions main/server/src/mill/main/server/Server.scala
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
package mill.main.server

import java.io._
import java.net.Socket
import java.net.{InetAddress, Socket}
import scala.jdk.CollectionConverters._
import org.newsclub.net.unix.AFUNIXServerSocket
import org.newsclub.net.unix.AFUNIXSocketAddress
import mill.main.client._
import mill.api.SystemStreams
import mill.main.client.ProxyStream.Output
Expand Down Expand Up @@ -48,7 +46,8 @@ abstract class Server[T](

while (
running && {
val serverSocket = bindSocket()
val serverSocket = new java.net.ServerSocket(0, 0, InetAddress.getByName(null))
os.write.over(serverDir / ServerFiles.socketPort, serverSocket.getLocalPort.toString)
try
interruptWithTimeout(() => serverSocket.close(), () => serverSocket.accept()) match {
case None => false
Expand All @@ -69,17 +68,6 @@ abstract class Server[T](
finally exitServer()
}

def bindSocket(): AFUNIXServerSocket = {
val socketPath = os.Path(ServerFiles.pipe(serverDir.toString()))
os.remove.all(socketPath)

val relFile = socketPath.relativeTo(os.pwd).toNIO.toFile
serverLog("listening on socket " + relFile + " " + os.pwd)
// Use relative path because otherwise the full path might be too long for the socket API
val addr = AFUNIXSocketAddress.of(relFile)
AFUNIXServerSocket.bindOn(addr)
}

def proxyInputStreamThroughPumper(in: InputStream): PipedInputStream = {
val pipedInput = new PipedInputStream()
val pipedOutput = new PipedOutputStream()
Expand Down
4 changes: 2 additions & 2 deletions main/util/src/mill/util/PromptLoggerUtil.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ package mill.util

private object PromptLoggerUtil {

private[mill] val defaultTermWidth = 119
private[mill] val defaultTermHeight = 50
private[mill] val defaultTermWidth = 99
private[mill] val defaultTermHeight = 25

/**
* How often to update the multiline status prompt on the terminal.
Expand Down
16 changes: 8 additions & 8 deletions main/util/test/src/mill/util/PromptLoggerTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ object PromptLoggerTests extends TestSuite {
promptLogger.close()

check(promptLogger, baos, width = 999 /*log file has no line wrapping*/ )(
"========================================================== TITLE =====================================================",
"======================================================================================================================",
"================================================ TITLE ===========================================",
"==================================================================================================",
// Make sure that the first time a prefix is reported,
// we print the verbose prefix along with the ticker string
"[1/456] my-task",
Expand All @@ -89,17 +89,17 @@ object PromptLoggerTests extends TestSuite {
// the double space prefix (since it's non-interactive and we don't need space for a cursor),
// the time elapsed, the reported title and ticker, the list of active tickers, followed by the
// footer
"[123/456] ================================================ TITLE ================================================= 10s",
"[123/456] ====================================== TITLE ======================================= 10s",
"[1] my-task 10s",
"======================================================================================================================",
"==================================================================================================",
"[1] WORLD",
// Calling `refreshPrompt()` after closing the ticker shows the prompt without
// the ticker in the list, with an updated time elapsed
"[123/456] ================================================ TITLE ================================================= 20s",
"======================================================================================================================",
"[123/456] ====================================== TITLE ======================================= 20s",
"==================================================================================================",
// Closing the prompt prints the prompt one last time with an updated time elapsed
"[123/456] ================================================ TITLE ================================================= 30s",
"======================================================================================================================",
"[123/456] ====================================== TITLE ======================================= 30s",
"==================================================================================================",
""
)
}
Expand Down
32 changes: 25 additions & 7 deletions runner/client/src/mill/runner/client/MillProcessLauncher.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
import mill.main.client.EnvVars;
import mill.main.client.ServerFiles;
import mill.main.client.Util;
import org.jline.terminal.Terminal;
import org.jline.terminal.TerminalBuilder;

public class MillProcessLauncher {

Expand Down Expand Up @@ -195,16 +193,36 @@ static List<String> readMillJvmOpts() {
return Util.readOptsFileLines(millJvmOptsFile());
}

static int getTerminalDim(String s) throws Exception {
Process proc = new ProcessBuilder()
.command("tput", s)
.redirectOutput(ProcessBuilder.Redirect.PIPE)
.redirectInput(ProcessBuilder.Redirect.INHERIT)
.redirectError(ProcessBuilder.Redirect.PIPE)
.start();
proc.waitFor();
return Integer.parseInt(new String(proc.getInputStream().readAllBytes()).trim());
}

static void writeTerminalDims(Path serverDir) throws Exception {
String str;
try {
str = java.lang.System.console() == null
? "0 0"
: getTerminalDim("cols") + " " + getTerminalDim("lines");
} catch (Exception e) {
str = "0 0";
}
Files.write(serverDir.resolve(ServerFiles.terminfo), str.getBytes());
}

public static void runTermInfoThread(Path serverDir) throws Exception {
Terminal term = TerminalBuilder.builder().dumb(true).build();

Thread termInfoPropagatorThread = new Thread(
() -> {
try {
while (true) {
Files.write(
serverDir.resolve(ServerFiles.terminfo),
(term.getWidth() + " " + term.getHeight()).getBytes());

writeTerminalDims(serverDir);
Thread.sleep(100);
}
} catch (Exception e) {
Expand Down

0 comments on commit b1f6be0

Please sign in to comment.