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

Community build: add cats and update cats-effect #10639

Merged
merged 9 commits into from
Dec 6, 2020

Conversation

griggt
Copy link
Contributor

@griggt griggt commented Dec 4, 2020

An alternative to #10587.

Updates cats-effect-2 and cats-effect-3 in the community build to latest upstream. Rather than modifying upstream to use withDottyCompat as was done in #10587, here we add the dependencies to the community build.

Projects added:

Name Reason
cats
discipline dependency of cats
discipline-munit dependency of cats and cats-effect-2
discipline-specs2 dependency of cats-effect-3
simulacrum-scalafix dependency of cats
cats-mtl transitive dependency of cats-effect-3
coop dependency of cats-effect-3

Fixes #10478

Copy link
Member

@smarter smarter left a comment

Choose a reason for hiding this comment

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

LGTM, this is super useful, thank you!

@smarter
Copy link
Member

smarter commented Dec 4, 2020

I find it weird that running JS tests can exhaust the JVM heap, wdyt @sjrd ? Tests should be running using node and not rhino right?

@griggt
Copy link
Contributor Author

griggt commented Dec 5, 2020

The GC thrashing seems to be triggered by:

[info] Full optimizing /src/dotty/community-build/community-projects/cats/tests/.js/target/scala-3.0.0-M2/cats-tests-test-opt
[info] Full optimizing /src/dotty/community-build/community-projects/cats/js/target/scala-3.0.0-M2/cats-js-test-opt
[info] Full optimizing /src/dotty/community-build/community-projects/cats/alleycats-tests/js/target/scala-3.0.0-M2/alleycats-tests-test-opt
[info] Fast optimizing /src/dotty/community-build/community-projects/cats/free/.js/target/scala-3.0.0-M3/cats-free-test-fastopt

@griggt
Copy link
Contributor Author

griggt commented Dec 5, 2020

Disabling full optimization for the Scala.js tests in cats seems to have solved the GC thrashing issue.

On another note, we'll need to fork all the added projects to dotty-staging if we want to merge this.

@smarter
Copy link
Member

smarter commented Dec 5, 2020

On another note, we'll need to fork all the added projects to dotty-staging if we want to merge this.

Ah yes, I think you should be able to create the forks since you're a member of the org.

@smarter
Copy link
Member

smarter commented Dec 5, 2020

Disabling full optimization for the Scala.js tests in cats seems to have solved the GC thrashing issue.

Until/unless this is merged upstream, we could do this in the community build config itself with set scalaJSStage in Global := FastOptStage so we don't have to diverge from upstream in the submodule.

@griggt
Copy link
Contributor Author

griggt commented Dec 5, 2020

since you're a member of the org.

am I? I only see collaborator access on dotty-staging/cats-effect, and am not seeing the option to fork to the dotty-staging org.

@smarter
Copy link
Member

smarter commented Dec 5, 2020

Oops I thought you did, you should have access now.

This is a dependency of cats
This is a dependency of cats and cats-effect-2
This is a dependency of cats-effect-3
This is a dependency of cats
Force fast optimization for Scala.js tests to avoid GC thrashing.
This is a transitive dependency of cats-effect-3
This is a dependency of cats-effect-3
@griggt griggt marked this pull request as ready for review December 5, 2020 23:57
Copy link
Member

@smarter smarter left a comment

Choose a reason for hiding this comment

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

Dotty CI / community_build_b (pull_request) Successful in 71m

This is now much longer than community_build_a, maybe we should move some other projects to _a to balance things out?

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.

cats-effect-* is broken in the community build after bump to 3.0.0-RC1
2 participants