Skip to content

Conversation

@romanowski
Copy link
Member

No description provided.

@romanowski romanowski force-pushed the graal-scala3 branch 3 times, most recently from 6f70b21 to 3520784 Compare April 2, 2022 14:59
@romanowski romanowski force-pushed the graal-scala3 branch 5 times, most recently from f489b88 to 61eb8da Compare April 11, 2022 10:04
@romanowski romanowski marked this pull request as ready for review April 11, 2022 10:04
Copy link
Contributor

@alexarchambault alexarchambault left a comment

Choose a reason for hiding this comment

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

I left some comments, but don't we want to move the graal processing stuff to its own project? It does pretty general things, that may also be used by projects others than Scala CLI.

Also, I'm not sure the global caching logic is fine with concurrency (file locks or atomic moves should be used to ensure it's fine). But are the computations it caches that slow? Couldn't a per-project cache work?

@romanowski romanowski force-pushed the graal-scala3 branch 2 times, most recently from fc1b66a to c40c47e Compare April 12, 2022 16:00
@romanowski
Copy link
Member Author

romanowski commented Apr 13, 2022

I left some comments, but don't we want to move the graal processing stuff to its own project? It does pretty general things, that may also be used by projects others than Scala CLI.

Wouldn't it be published with Scala CLI anyway? I can also do that once its stabilize and it can be another lib that we can test publishing from Scala CLI on.scala-cli

Also, I'm not sure the global caching logic is fine with concurrency (file locks or atomic moves should be used to ensure it's fine). But are the computations it caches that slow? Couldn't a per-project cache work?

I think I can change it back to process everything every time (TempCache) and we can investigate a proper caching in follow up PR (actually, this can make a nice task for someone outside main Scala CLI team)

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.

2 participants