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

Workflows: use native for community tests in CI #4758

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kitbellew
Copy link
Collaborator

No description provided.

@kitbellew kitbellew force-pushed the 4758 branch 3 times, most recently from 82d040a to 4b5b3c6 Compare January 29, 2025 01:38
@kitbellew
Copy link
Collaborator Author

@tgodzik @jchyb i was working on this purely out of curiosity, and i don't understand the results. if you compare this latest test run using native (forgetting for a second that it failed), it is slower than the previous run which used jvm:

native: https://github.com/scalameta/scalafmt/actions/runs/13086461314
jvm: https://github.com/scalameta/scalafmt/actions/runs/13085684687

native build takes 8 minutes, jvm a little over 1.

jvm test of scala3: 11 minutes (https://github.com/scalameta/scalafmt/actions/runs/13085684687/job/36516501088)
native test of scala3: 16 minutes (https://github.com/scalameta/scalafmt/actions/runs/13086461314/job/36518171422)

so, native is only 2 minutes faster, excluding the build step, which is hardly earth-shattering. both use the same approach, which includes futures for parallelism. i wonder...

@tgodzik
Copy link
Contributor

tgodzik commented Feb 2, 2025

I wouldn't expected a large difference without anyone more competent than me looking into optimizing it. Though, shaving off 2 minutes is still good in my book. It's not so useful if you have to build the native binaries yourself.

Potentially, we could build the binaries, upload them and download to run on a specific suite.

@jchyb
Copy link
Contributor

jchyb commented Feb 3, 2025

It should be a little faster after merging #4789 (the previous very positive looking comparisons were made using 2.12, where that library was not necessary), but there's not much we can do about the long build times if we want to keep the runtime fast.

@kitbellew
Copy link
Collaborator Author

It should be a little faster after merging #4789 (the previous very positive looking comparisons were made using 2.12, where that library was not necessary), but there's not much we can do about the long build times if we want to keep the runtime fast.

i will review it, and it's generally useful, but we switched to using futures, so it will not have effect but perhaps we could review how we use futures.

@jchyb
Copy link
Contributor

jchyb commented Feb 3, 2025

Ah, I see, it might not improve things much then

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.

3 participants