Skip to content

Add memory config to checkstyle#20198

Merged
bot-gradle merged 8 commits intogradle:masterfrom
breskeby:add-memory-config-to-checkstyle
Mar 18, 2022
Merged

Add memory config to checkstyle#20198
bot-gradle merged 8 commits intogradle:masterfrom
breskeby:add-memory-config-to-checkstyle

Conversation

@breskeby
Copy link
Copy Markdown
Contributor

@breskeby breskeby commented Mar 17, 2022

Fixes Out of memory issues when running checkstyle against large sourceSets

Context

With having ported the Checkstyle task to use the worker api and running in a separate java process (#20069) we see OOM problems
with our elasticsearch build when using the latest gradle master (7.5 snapshot)
This PR makes the heap size for checkstyle tasks configurable to not be forced to
rely on jdk defaults.

The property namings are similar to how memory settings are named in the Test task.

Contributor Checklist

  • Review Contribution Guidelines
  • Make sure that all commits are signed off to indicate that you agree to the terms of Developer Certificate of Origin.
  • Make sure all contributed code can be distributed under the terms of the Apache License 2.0, e.g. the code was written by yourself or the original code is licensed under a license compatible to Apache License 2.0.
  • Check "Allow edit from maintainers" option in pull request so that additional changes can be pushed by Gradle team
  • Provide integration tests (under <subproject>/src/integTest) to verify changes from a user perspective
  • Provide unit tests (under <subproject>/src/test) to verify logic
  • Update User Guide, DSL Reference, and Javadoc for public-facing changes
  • Ensure that tests pass sanity check: ./gradlew sanityCheck
  • Ensure that tests pass locally: ./gradlew <changed-subproject>:quickTest

Gradle Core Team Checklist

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation
  • Recognize contributor in release notes

With having ported the Checkstyle task to use the worker api we see OOM problems
with our elasticsearch build when using the latest gradle master (7.5 snapshot)
This PR makes the heap size for checkstyle tasks configurable to not be forced to
rely on jdk defaults.
The property namings are similar to how memory settings are named in the Test task.

Signed-off-by: Rene Groeschke <rene@elastic.co>
Signed-off-by: Rene Groeschke <rene@elastic.co>
@breskeby breskeby force-pushed the add-memory-config-to-checkstyle branch from e4518df to 5a9392e Compare March 17, 2022 10:09
@wolfs wolfs added this to the 7.5 RC1 milestone Mar 17, 2022
Signed-off-by: Rene Groeschke <rene@elastic.co>
@big-guy
Copy link
Copy Markdown
Member

big-guy commented Mar 17, 2022

@bot-gradle test this

@bot-gradle
Copy link
Copy Markdown
Collaborator

OK, I've already triggered the following builds for you:

…Options.java

Apply suggested javadoc tweaks

Co-authored-by: Stefan Wolf <wolf@gradle.com>
Signed-off-by: Rene Groeschke <rene@elastic.co>
@breskeby breskeby force-pushed the add-memory-config-to-checkstyle branch from 19f0999 to 8ed734d Compare March 18, 2022 11:50
@asodja asodja force-pushed the add-memory-config-to-checkstyle branch from 290e19f to 831fbaf Compare March 18, 2022 12:52
Copy link
Copy Markdown
Member

@asodja asodja left a comment

Choose a reason for hiding this comment

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

LGTM

@asodja
Copy link
Copy Markdown
Member

asodja commented Mar 18, 2022

@bot-gradle test and merge

@bot-gradle
Copy link
Copy Markdown
Collaborator

OK, I've already triggered a build for you.

@bot-gradle bot-gradle merged commit 06784ad into gradle:master Mar 18, 2022
@asodja
Copy link
Copy Markdown
Member

asodja commented Mar 18, 2022

Thanks for your contribution!

@big-guy big-guy added the from:contributor PR by an external contributor label Apr 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

from:contributor PR by an external contributor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants