-
-
Notifications
You must be signed in to change notification settings - Fork 347
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Windows client/server improvements #262
Conversation
@robby-phd if the problem is just that it hangs for a long time confusingly, would an acceptable workaround be to log something to the console when initializing rt.jar for the first time? That seems like an easy "fix" and is something we already do when e.g. we spend time initializing the Zinc compiler interface (which takes 10s of seconds, only once). I'm not very comfortable with the ad-hoc parsing code we have here; it means that any user code which has the |
@lihaoyi Ah yes... I forgot that mill supports passing args to user code; that can be taken into account. On the leaving it hanging, as I mentioned, the time can vary significantly, sometimes fast and sometimes very long (minutes); if you think that's ok for now, I'll limit the PR to just the first problem. |
4df2ae9
to
20a197d
Compare
@robby-phd yeah I think hanging is ok as long as it is properly messaged to the user so they know what to expect ("currently preparing Java 9 Scala integration jar, this may take a minute or two...") |
Hmm.. getting strange errors locally now when I use mill built using
@lihaoyi any idea why? |
@lihaoyi Turns out it's an unrelated new bug introduced after 0.1.7-8-b913c6 (that is used in ci); I recreated the issue on travis and appveyor by upgrading the ci test to use 0.1.7-19-680364 (latest, which I was using locally). Anyway, let me know if the changes in this PR now looks ok to you and I'll merge. |
@robby-phd looks good to me |
This PR attempts to fix a couple of issues with Windows client/server:
Failure to connect to server manifested as IOException (first commit)
This is addressed by
re-attempting the connection andhard shutdown usingSystem.exit
after several attempts(currently, the client exits with code 1without printing any message).
The more pressing issue is the unpredictable performance under Java 9 when starting mill client/server for the first time (second commit)
Unfortunately, mill client/server under Windows with Java 9 (or above) runs unpredictably
when run for the first time. This is because exporting rt.jar on the server side can somehow
be non-deterministically slow after displaying:
This can take a minute (or more, or fast), but then it will eventually make progress.
Below is the jstack dump as a reference whenever it seems to "hang" before making progress
Of interest here is
Thread-1
where it can spend a long time exporting rt.jar (I sampled this several times during such a hang and they all looked similar, i.e.,Thread-1
is in the process of exporting rt.jar).One workaround is to ask user to run mill interactively first to trigger rt.jar caching
(I find this rather unacceptable for users).
On the other hand, triggering the rt.jar caching early when mill first starts seems to
avoid this issue.
This PR does that for interactive/non-interactive modes(needed only for client/server on Windows using Java 9+ but done for all for UX consistency).
Unfortunately, the changes are somewhat not ideal because we want to keep millclient Java only (to avoid slowdown caused by class loading Scala); the main issue
here is that eagerly caching rt.jar requires it to know mill's home, which implies
parsing CLI arguments early on.