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

Speed up build #342

Closed
fwbrasil opened this issue May 9, 2024 · 6 comments · Fixed by #450
Closed

Speed up build #342

fwbrasil opened this issue May 9, 2024 · 6 comments · Fixed by #450

Comments

@fwbrasil
Copy link
Collaborator

fwbrasil commented May 9, 2024

I've noticed Kyo's codebase is taking longer to compile for some time now. It seems the linting compiler flags added some time ago are the main culprit. Removing these flags produced a significant speedup in an initial test:

"-Wvalue-discard",
"-Wunused:all",
"-language:strictEquality"
@hearnadam
Copy link
Collaborator

I think it's worth removing the formatting on compile. We can add that on test... scalafmt can be quite slow.

@fwbrasil
Copy link
Collaborator Author

/bounty $50

Copy link

algora-pbc bot commented May 21, 2024

💎 $50 bounty • Kyo

Steps to solve:

  1. Start working: Comment /attempt #342 with your implementation plan
  2. Submit work: Create a pull request including /claim #342 in the PR body to claim the bounty
  3. Receive payment: 100% of the bounty is received 2-5 days post-reward. Make sure you are eligible for payouts

Thank you for contributing to getkyo/kyo!

Add a bountyShare on socials

@fwbrasil
Copy link
Collaborator Author

I've tried adding the new option ThisBuild / usePipelining := true that was recommended on reddit but it seems to infer a wrong compilation order and fails:

[error] -- [E008] Not Found Error: /Users/fwbrasil/workspace/kyo/kyo-scheduler/jvm/src/main/scala/kyo/scheduler/package.scala:6:11
[error] 6 |import kyo.stats.internal.StatsRegistry
[error]   |       ^^^^^^^^^
[error]   |       value stats is not a member of kyo

Removing the flag makes the build work again.

@regiskuckaertz
Copy link

@kyri-petrou
Copy link
Contributor

I know this doesn't solve the issue, but the build should be considerably faster starting with Scala 3.5.x since they fixed unused imports linting performance. Note that 3.5.0-RC1 is already out already in case you want to test it

fwbrasil added a commit that referenced this issue May 31, 2024
- Split root project into `kyoJVM` and `kyoJS`. The JVM project is
selected by default, this way the regular development cycle is faster
but there's the downside that we might not catch issues in JS before CI.
It seems a reasonable tradeoff since it's possible to run the tests
manually by selecting the `kyoJS` project.
- Avoid the dependency on `kyo-core` tests in other modules. All other
modules end up stuck waiting for the core tests to compile because of
the dependency. I had to duplicate some of the code in `KyoTest` into
the other modules to achieve that.
- Move `scalafmt` from the build to a commit hook.
- Remove `-Wunused:all` compiler option. It significantly increases the
compilation times for tests. We can attempt to add it back once we're on
Scala 3.5, which seems to [improve build
times](#342 (comment))
with the option enabled.

Results with a cold JVM:

**Main**
```
$ sbt ";clean;compile"
[success] Total time: 24 s, completed May 30, 2024, 1:58:13 PM
$ sbt ";clean;test:compile"
[success] Total time: 58 s, completed May 30, 2024, 1:59:58 PM
$ sbt ";clean;test"
[success] Total time: 88 s (01:28), completed May 30, 2024, 2:01:45 PM
```

**This PR (default `kyoJVM` only)**
```
$ sbt ";clean;compile"
[success] Total time: 14 s, completed May 30, 2024, 1:54:29 PM
$ sbt ";clean;test:compile"
[success] Total time: 27 s, completed May 30, 2024, 1:55:51 PM
$ sbt ";clean;test"
[success] Total time: 39 s, completed May 30, 2024, 1:56:59 PM
```

I won't close #342 since there's
always opportunity to optimize the build.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants