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

setup-java post action caching for v4 is slower than v3 #601

Closed
2 of 5 tasks
htpaf opened this issue Feb 29, 2024 · 8 comments
Closed
2 of 5 tasks

setup-java post action caching for v4 is slower than v3 #601

htpaf opened this issue Feb 29, 2024 · 8 comments
Assignees
Labels
bug Something isn't working

Comments

@htpaf
Copy link

htpaf commented Feb 29, 2024

Description:
When switching from setup-java v3 to v4 the post run part is significantly slower.

Task version:
v4

Platform:

  • Ubuntu
  • macOS
  • Windows

Runner type:

  • Hosted
  • Self-hosted

Repro steps:

Slow:

jobs:
  verify:
    runs-on: ubuntu-22.04
    steps:
      - uses: actions/checkout@v4
        with:
          fetch-depth: 0
      - uses: actions/setup-java@v4
        with:
          java-version: '11'
          distribution: 'temurin'
          cache: 'maven'
      - name: run maven
        run: |
          mvn -B --settings .mvn/settings.xml verify
        shell: bash

image


Fast:

jobs:
  verify:
    runs-on: ubuntu-22.04
    steps:
      - uses: actions/checkout@v4
        with:
          fetch-depth: 0
      - uses: actions/setup-java@v3
        with:
          java-version: '11'
          distribution: 'temurin'
          cache: 'maven'
      - name: run maven
        run: |
          mvn -B --settings .mvn/settings.xml verify
        shell: bash

image

Expected behavior:
v4 should have the same speed as v3.
Both executions had no cache hits and the amount of data was approx 30MB.

Actual behavior:
v4 is in this case orders of magnitude slower than v3.

Additional information:

I have seen that actions setup-node actions/setup-node#878
and setup-ruby ruby/setup-ruby#543 have had the same problem.
It seems that it had to do with the upgrade of the actions/cache version.

I think their solution was to exit the process manually after caching is complete as otherwise the job simply hangs until some timeout.

After looking at this repo for a very short time my guess is that perhaps somewhere
here ->

core.info(`Cache saved with the key: ${primaryKey}`);

a process.exit(0) could solve the problem (as I said, I'm only guessing based on the node and ruby solutions).

@htpaf htpaf added bug Something isn't working needs triage labels Feb 29, 2024
@HarithaVattikuti
Copy link
Contributor

Hello @htpaf
Thank you for creating this issue. We will investigate it and get back to you as soon as we have some feedback.

@aparnajyothi-y
Copy link
Contributor

Hello @htpaf, Thank you once again for creating this issue.
We have investigated about the issue and able to reproduce the issue but there is no major changes in setup-java that affect the performance time from v3 to v4 other than node 20 upgrade. We tried multiple scenarios and found that the this long time duration is due to the dependencies installation time for node20. As per the node.js official documentation for node 20, we found that from node v19, Global instance of Agent uses keep-alive by default for all HTTP client requests .And the current usage is - HttpClient implementation uses globalAgent when RequestOptions.keepAlive is set to false.
There is a PR is the fix for the same is merged to actions/toolkit repository. The issue resolves after RequestOptions.keepAlive is applied properly on node20 runtime. Please find the screenshot for reference.

image

We will update you once the changes released. Thank you.

@aparnajyothi-y aparnajyothi-y self-assigned this Mar 4, 2024
@htpaf
Copy link
Author

htpaf commented Mar 7, 2024

Thank you for the update and work on this issue @aparnajyothi-y .

This issue is related to if not a duplicate of #591.

I read the comments over there and @fniephaus mentions the toolkit problem.

I am thinking that keepAlive setting might solve the problem but the fix done for other actions such as this setup-ruby fix seems relevant. It would seem better to explicitly complete/end the request not wait for a timeout, whose actual value could be changed upstream at any time, and then have better guarantees and not get regressions.

Does that make sense @aparnajyothi-y ?

@htpaf
Copy link
Author

htpaf commented Mar 8, 2024

The toolkit PR is released in
this PR and
setup-java uses the 2.2.1 version in this PR.

A release PR for setup-java from action:main to main branch is probably what makes it generally available in v4.

It should be possible to use the setup-java PR branch to test already... but it is a fork so probably not until it is in this repository.

@aparnajyothi-y
Copy link
Contributor

Hello @htpaf, Thank you for the information, We will get back you soon on this.

@htpaf
Copy link
Author

htpaf commented Mar 11, 2024

I tried with setup-java@main (given this merged PR) and got:

image

This looks much better! Great work!

So yes @aparnajyothi-y looking forward to when the v4 tag will contain this update.

@aparnajyothi-y
Copy link
Contributor

Hello everyone, The actions/toolkit#1572 released and updated actions/http-client version to 2.2.1 as part of setup-java v4.2.0 release. Tested from our end, the time difference in the post run process in v4. Please check and confirm to proceed to close this issue.

@htpaf
Copy link
Author

htpaf commented Mar 18, 2024

Verified with 99b8673 corresponding to https://github.com/actions/setup-java/releases/tag/v4.2.1

So all good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants