Skip to content
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

Use Scala-Native for Mill Client (1000USD Bounty) #4016

Open
lihaoyi opened this issue Nov 24, 2024 · 21 comments
Open

Use Scala-Native for Mill Client (1000USD Bounty) #4016

lihaoyi opened this issue Nov 24, 2024 · 21 comments
Labels

Comments

@lihaoyi
Copy link
Member

lihaoyi commented Nov 24, 2024


From the maintainer Li Haoyi: I'm putting a 1000USD bounty on this issue, payable by bank transfer on a merged PR implementing this.


Sister ticket to #4007.

The Mill client contains ~24 Java files with zero third party dependencies:

We should try to see if we can port it all to Scala-Native for improved startup performance and to allow us to use Scala libraries in the client. e.g. using MainArgs in the Mill client would alleviate the -i must be passed as the first argument restriction which is very annoying

The success criteria for this ticket is a PR which (a) translates all Mill client code from Java to Scala (b) cross-compiles the Mill client code using Scala-Native and (c) bundles the Scala native launcher together with the Mill server jar so the client is able to launch the server.

@lihaoyi lihaoyi added the bounty label Nov 24, 2024
@Quafadas
Copy link
Contributor

I'd be tempted to dot take a tilt at this, but I'll probably be slow.

@scarf005
Copy link

scarf005 commented Nov 24, 2024

i'm also interested in working on this, but pointers on how to incrementally migrate would be welcome: https://discord.com/channels/632150470000902164/1295786923410984981/1310137269801320498

@Quafadas
Copy link
Contributor

@scarf005 I'll leave it to you for the time being - if you're willing to publish your PR, we can try and work through it together, but it sounds to me as though you're doing the same things I would have been trying to do.

My knowledge is not more detailed than what Haoyi set out above right now.

@scarf005
Copy link

(b) cross-compiles the Mill client code using Scala-Native and (c) bundles the Scala native launcher together with the Mill server jar so the client is able to launch the

@lihaoyi sorry, but could this be explained in more detail? still quite new to mill, i've rewritten runner.client at https://github.com/scarf005/mill/tree/refactor/scala-native-mill-client but unsure which command to run to ensure (b) and (c) works correctly.

@Quafadas
Copy link
Contributor

@scarf005 If you try running this command;

mill show runner.client.compile

and check the compile.dest target dir. I guess, that it will be empty. Did you make the client module a ScalaModule? Otherwise, it will ignore *.scala.

@Quafadas
Copy link
Contributor

@scarf005 Just want to check - are you still looking at this?

@scarf005
Copy link

yes, i think i could work more on weekends...
tho it might not work well as planned since i'm new on this!

@Quafadas
Copy link
Contributor

@scarf Cool - if you want to collaborate on it and split the bounty, it could be fun - otherwise I'll leave you to it ... no pressure from me either way.

tho it might not work well as planned since i'm new on this!

it's open source so hopefully it's fun :-).

If you decide to drop it at some point, just please let me know?

@scarf005
Copy link

alright, will share how it goes here. btw - my handle is scarf005 not scarf

@lihaoyi
Copy link
Member Author

lihaoyi commented Nov 27, 2024

Just for some additional context, the long-term plan for this issue and #4007 is we will likely want both: initially to do a comparative analysis of Scala-Native vs Graal-Native approaches to the problem, and then either picking the Scala-Native version OR using Scala re-implementation on the JVM with Graal-Native. In either case we would need both tickets to be completed

@scarf005
Copy link

scarf005 commented Dec 1, 2024

if you want to collaborate on it and split the bounty, it could be fun - otherwise I'll leave you to it ... no pressure from me either way.

tho it might not work well as planned since i'm new on this!

it's open source so hopefully it's fun :-).

If you decide to drop it at some point, just please let me know?

@Quafadas I'm fine with collaborating, but I think won't have much time to work on this except weekends. consider me dropped for now. feel free to reference my branch (most of the changes are AI generated anyways lol), I was stuck on actually running the tests

@ayewo
Copy link
Contributor

ayewo commented Dec 1, 2024

I'd like to take a stab at this.

@Quafadas
Copy link
Contributor

Quafadas commented Dec 1, 2024

Here's my work so far.

#4057 . Currently, I think this looks like it might be very hard. I'm going to head over the Scala Native, and ask about how equivalent to the JVM file handling they are / supposed to be / think they are at the moment to see if it's a "me" thing...

@Quafadas
Copy link
Contributor

Quafadas commented Dec 1, 2024

@lihaoyi

Although it's possible that I've mucked up something simple, I had the tests passing before switching mill to native modules, and my (limited test) ports appear to pass if I run them on the JVM rather than native. I'm asking around in the Scala Native discord, but, if I'm right, a native client could entail a quite low level re-write that changes the client locking mechanisms and the streaming between log -> terminal mechanisms.

I guess my question is how far would you want to push this and what the burden of proof in either direction would be? It could get quite high risk?

@lihaoyi
Copy link
Member Author

lihaoyi commented Dec 1, 2024

The bounty is for full compatibility, including the logging and locking mechanisms. If you can't get that working that's fine, perhaps others will make more progress in future.

Burden of proof is same as any other PR, code review and testing. In this case, all existing tests should pass with the new launcher, both unit tests for the launcher and integration tests for Mill. Spawning a new server every run would not pass tests

@Quafadas
Copy link
Contributor

Quafadas commented Dec 1, 2024

Good, agreed. I think I should drop this for the time being. From what I can tell, to get this over the line is a far larger undertaking than I am in a position to credibly work through. Apologies for that.

I do think the PR that I put up could provide a good starting point for someone and I'm happy to answer question on it. Much of the infrastructure is in place.

@Quafadas
Copy link
Contributor

Quafadas commented Dec 8, 2024

This repository painstakingly rebuilt the client in a cleanroom with all but two unit tests passing.

https://github.com/Quafadas/mill_client

I believe it to be a SN bug (tests pass if you switch Scala CLI to JVM platform) and have asked for help.

@lihaoyi
Copy link
Member Author

lihaoyi commented Dec 9, 2024

Sounds great @Quafadas !

@Quafadas
Copy link
Contributor

Quafadas commented Jan 12, 2025

@lihaoyi I've seen that you've merged a "native client" style solution based on graal? Congratulations- very exciting :-).

LeeTibbert has indicated he might have time to look into the (stalled) native client port, but my perception is that the case for continuing here is greatly weakened, as mill already has already gained "native start" capability. Is that fair? I'm reluctant to ask for help (from skilled and time scarce people) that would be required to help get this over the line, given the marginal benefit appears limited?

Is that fair? I've pointed Mr Tibbert to this conversation and perhaps he chimes in too...

@lihaoyi
Copy link
Member Author

lihaoyi commented Jan 12, 2025

I'd still be willing to pay out the bounty for a scala-native implementation, maybe skipping the coursier jvm downloader since coursier isnt on scala native yet.

Potential benefits include

  • smaller binaries (graal is currently 20-60mb without/with coursier included),
  • cross building (would be nice to not need N separate jobs for each cpu/os combo)
  • windows-arm support (graal native image does not support this right now)
  • as a stress test for scala native itself, which would force us to flesh it out further and help that project

The need is not as acute as it was before we got graal native image working, but the bounty remains open

@Quafadas
Copy link
Contributor

Quafadas commented Feb 7, 2025

@lihaoyi I believe the next version of scala native passes the client tests in the cleanroom I setup (thanks Mr Tibbert!). So that part would appear unblocked.

So... let's carry on ? I believe, that it should be possible, that most of the heavy lifting can come out of 2 small PRs that I think should be self contained and independant.

  1. Change client tests to utest and .scala
  2. Port all client .java files to .scala

This shouldn't muck anything else up. I think that means it's all low risk to this point?

After that, I think it's a case of cross building the client for JVM / native in such a way that it doesn't interfere with what you've already done on the Graal side...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants