Optimize startup time using TCP sockets instead of junixsocket and tput instead of jline
#4009
Conversation
lihaoyi
changed the title
tput instead of jline
lihaoyi
changed the title
tput instead of jlinetput instead of jline
| @@ -48,7 +46,8 @@ abstract class Server[T]( | |||
|
|
|||
| while ( | |||
| running && { | |||
| val serverSocket = bindSocket() | |||
| val serverSocket = new java.net.ServerSocket(0) | |||
There was a problem hiding this comment.
This binds to 0.0.0.0 for me, so it is listening to the world, including external network. At least on my machine.
So, better IMO:
val serverSocket = new java.net.ServerSocket()
serverSocket.bind(new java.net.InetSocketAddress("127.0.0.1",0))
I suggesting the IP 127.0.0.1 over 'localhost', because localhost sometimes resolves to IP6 vs IP4, creating odd miss-matches.
There was a problem hiding this comment.
Looks like I forgot to pass in the host entirely, but yeah I can use 127.0.0.1
| 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("localhost", port); |
There was a problem hiding this comment.
I recommend '127.0.0.1' over localhost. I had multiple times where using 'localhost' gives mixups between IP4 and IP6.
Like one process resolves it to IP4 and the other process to IP6 and then they fail to connect each other.
Or cases where resolving localhost added extra mill-seconds for weird reasons.
| catch { case e: FileAlreadyExistsException => /* someone else already did this */} | ||
| catch { | ||
| case e: FileAlreadyExistsException => /* someone else already did this */ | ||
| } | ||
| } |
There was a problem hiding this comment.
@lihaoyi this file also got same changes in my PR while fixing linting error should i remove the changes from there or rebasing and merge will automatically fix this
The
AFUNIXServerSocketlibrary 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
UnixDomainSocketAddressstarting 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 nowjline.terminal.Terminal.getSizealso adds a few hundred milliseconds, so we instead use the sametputcommand that Ammonite uses which takes <10ms.tputisn'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 experienceTogether these two changes cuts down the time taken for a hot
time ./mill versionon the Mill repo from ~1.05s to ~0.24s