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

Scala Native #3057

Merged
merged 78 commits into from
Sep 21, 2022
Merged

Scala Native #3057

merged 78 commits into from
Sep 21, 2022

Conversation

armanbilge
Copy link
Member

Closes #1302.

@armanbilge armanbilge marked this pull request as draft June 27, 2022 23:35
Comment on lines +52 to +56
@extern
@nowarn
private[std] object sysrandom {
def getentropy(buf: Ptr[Byte], buflen: CSize): Int = extern
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works on Linux and macOS. I reckon that's plenty good for now. Anyone can implement SecureRandom for themselves to suit their requirements.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We definitely need to write up some serious words about the limitations of the current implementation. :-)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. But note that anyone who tries to use this on an unsupported platform will get an error at linking time, before runtime. So it's not evil, just annoying.

Comment on lines +348 to +350
.nativeSettings(
libraryDependencies += "io.github.cquiroz" %%% "scala-java-time" % "2.4.0"
)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think Scala Native has the same artifact size concerns as Scala.js, so I reckon we should ship java.time support out-of-the-box.

Comment on lines 25 to 27
def blocking[A](thunk: => A): IO[A] =
apply(thunk)
// do our best to mitigate blocking
IO.cede *> apply(thunk) <* IO.cede
Copy link
Member Author

@armanbilge armanbilge Sep 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Downstream libraries are riddled with blocking calls (e.g. DNS resolution in ip4s, file I/O in fs2). Actually even IO.println is blocking! Since we don't have a blocking threadpool to run them on, this seems like the best we can do to mitigate their effects.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually we can do even better here: this should be apply(thunk).guarantee(IO.cede). We don't want to skip that cede even if we errored.

@djspiewak
Copy link
Member

So this is all in series/3.3.x now right? How do we untangle the git gordian knot?

@armanbilge
Copy link
Member Author

armanbilge commented Sep 19, 2022

No, this is not really in series/3.3.x. Actually pulling that off would have been too painful because Scala Native requires Scala 3.1+ and the JVM/JS series/3.3.x is stuck on Scala 3.0. Hence my "backport" PR #3138 really started a parallel branch series/3.3.x-native. That branch can simply be abandoned.

Yes, sorry, not ideal. But I wasn't prepared to let this die-on-the-vine like my Env PR from *checks watch* ...

@djspiewak
Copy link
Member

Yes, sorry, not ideal. But I wasn't prepared to let this die-on-the-vine like my Env PR from checks watch ...

whistles innocently

Either way, given that this has now been released, I think it's best if we get it into series/3.x to form a part of 3.4. :-)

@armanbilge
Copy link
Member Author

Ok, @djspiewak, let's land this one :) tbh I was quite sad to ship this out in 3.3.14 without your review, so I'm looking forward to that 😇

Question: do we want to add a macOS CI job? There a couple places where we directly call native APIs (high-precision time and cryptographically strong random numbers). They are all supported on Linux/macOS, but in theory we should be careful.

@djspiewak
Copy link
Member

Question: do we want to add a macOS CI job? There a couple places where we directly call native APIs (high-precision time and cryptographically strong random numbers). They are all supported on Linux/macOS, but in theory we should be careful.

I hate to say it, but we should be careful. :-( So that means one more matrix entry.

@armanbilge
Copy link
Member Author

Something I haven't done in this PR is setup the test harness so we can run the IOAppSpec with Native executables. It's possible by the same trick we use to run the IOAppSpec for JS. So just a yak I'd be happy to put off 😆

@djspiewak djspiewak added this to the v3.4.0 milestone Sep 20, 2022
Copy link
Member

@djspiewak djspiewak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly just worried about the lack of documentation and caveats!

Comment on lines +204 to +205
lazy val keepAlive: IO[Nothing] =
IO.sleep(1.hour) >> keepAlive
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we actually need this with Scala Native?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe so! The whole point of this is to keep an IO.never program alive. In the IO.never case the EC queue will be empty. In theory we could just have the main thread go into a permanently parked state in this situation.

But in practice the EC actually has to stop running if its queues are empty so that it can yield to the Scala Native global EC, to make test frameworks work correctly. See also the doc comment on poll(...).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fascinating. It's identical code but actually handled in an entirely different way.

Comment on lines +27 to +29
def blocking[A](thunk: => A): IO[A] =
// do our best to mitigate blocking
IO.cede *> apply(thunk).guarantee(IO.cede)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😱

import scala.concurrent.CancellationException
import scala.concurrent.duration._

/**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we write some words in here which clarifies 1) the experimental nature of things, and 2) the limitations imposed by this particular IOApp? It's fair to call out to EpollCat for now since that's the only usable runtime.

Copy link
Member Author

@armanbilge armanbilge Sep 20, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep can write some doc. Besides the blocking issue I wouldn't really say IOApp is limited. Anything you can do in Scala Native today works fine with this IOApp. If you want to do async I/O with JDK APIs, tough, because they are not implemented anyway, but that's not this IOApps fault.

It's fair to call out to EpollCat for now since that's the only usable runtime.

It's not actually. It's only necessary for fs2.io.net and thus ember or skunk etc.

I have a CurlApp in http4s-curl that's perfectly good if the only non-blocking I/O you need are HTTP requests (this is sufficient to implement a Lambda runtime btw). Also that runtime is theoretically capable of polling on arbitrary file descriptors, but I have not exposed that.

I also just wrote an io_uring-based runtime in armanbilge/fs2-io_uring#2. But that's still a bit premature :)

Finally, see also #3153 (comment) where we discussed some interesting native applications built with vanilla IOApp, including CLIs and an LSP.

Comment on lines +23 to +24
@nowarn("msg=never used")
def calculateTracingEvent(key: Any): TracingEvent = null
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be fun™…

Comment on lines +52 to +56
@extern
@nowarn
private[std] object sysrandom {
def getentropy(buf: Ptr[Byte], buflen: CSize): Int = extern
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We definitely need to write up some serious words about the limitations of the current implementation. :-)

package cats.effect

trait DetectPlatform {
def isWSL: Boolean = System.getProperty("os.version").contains("-WSL")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How are system properties handled on Scala Native?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No idea what this would say for WSL. But I believe it does attempt to populate them as the JVM would.

@djspiewak djspiewak merged commit bda6ac6 into typelevel:series/3.x Sep 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Experiment with scala native
3 participants