diff --git a/.github/env/Linux/gradle.properties b/.github/env/Linux/gradle.properties deleted file mode 100644 index 810c16c9356..00000000000 --- a/.github/env/Linux/gradle.properties +++ /dev/null @@ -1,18 +0,0 @@ -# TODO (deephaven-core#639): Customize gradle settings per-CI job -# -# https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners#supported-runners-and-hardware-resources -# GitHub hosted linux runner environment has 2 CPUs and 7G RAM. -# -# testParallel seems to run poorly on GH Actions, and is unable to take advantage it seems of 2 -# workers. -# -# Our engine-table tests currently request 6g of heap (engine/table/build.gradle); this means we can't have more than -# 1 worker at a time. - -org.gradle.parallel=false -org.gradle.workers.max=1 - -# Our CI JDKs should be pre-provisioned and invoked correctly, -# we shouldn't rely on gradle for any of this logic. -org.gradle.java.installations.auto-download=false -org.gradle.java.installations.auto-detect=false diff --git a/.github/scripts/gradle-properties.sh b/.github/scripts/gradle-properties.sh new file mode 100755 index 00000000000..d271ad29f0c --- /dev/null +++ b/.github/scripts/gradle-properties.sh @@ -0,0 +1,50 @@ +#!/usr/bin/env bash + +set -o errexit +set -o pipefail +set -o nounset + +# We know the gradle daemon takes 1GiB (see gradle.properties) +DAEMON_BYTES=1073741824 + +# We'll leave some buffer space for other system resources / processes +OTHER_BYTES=2147483648 + +TOTAL_SYSTEM_BYTES="$(free --bytes | grep Mem | awk -F " " '{print $2}')" + +# This is accounting for "worst case", assuming every single worker is using the theoretical maximum. +# Currently, engine/table/build.gradle sets a heap size of 6GiB, so that's the maximum. +PER_WORKER_BYTES=6442450944 + +# See https://github.com/gradle/gradle/issues/14431#issuecomment-1601724453 for why we need to have this sort of logic +# here +MAX_WORKERS="$(( (TOTAL_SYSTEM_BYTES - DAEMON_BYTES - OTHER_BYTES) / PER_WORKER_BYTES ))" +MAX_WORKERS="$(( MAX_WORKERS > 0 ? MAX_WORKERS : 1 ))" + +# A bit zealous, but this handles the https://github.com/actions/setup-java output +JAVA_INSTALL_PATHS="" +JAVA_INSTALL_PATHS="${JAVA_INSTALL_PATHS}${JAVA_HOME_8_X64:+$JAVA_HOME_8_X64,}" +JAVA_INSTALL_PATHS="${JAVA_INSTALL_PATHS}${JAVA_HOME_9_X64:+$JAVA_HOME_9_X64,}" +JAVA_INSTALL_PATHS="${JAVA_INSTALL_PATHS}${JAVA_HOME_10_X64:+$JAVA_HOME_10_X64,}" +JAVA_INSTALL_PATHS="${JAVA_INSTALL_PATHS}${JAVA_HOME_11_X64:+$JAVA_HOME_11_X64,}" +JAVA_INSTALL_PATHS="${JAVA_INSTALL_PATHS}${JAVA_HOME_12_X64:+$JAVA_HOME_12_X64,}" +JAVA_INSTALL_PATHS="${JAVA_INSTALL_PATHS}${JAVA_HOME_13_X64:+$JAVA_HOME_13_X64,}" +JAVA_INSTALL_PATHS="${JAVA_INSTALL_PATHS}${JAVA_HOME_14_X64:+$JAVA_HOME_14_X64,}" +JAVA_INSTALL_PATHS="${JAVA_INSTALL_PATHS}${JAVA_HOME_15_X64:+$JAVA_HOME_15_X64,}" +JAVA_INSTALL_PATHS="${JAVA_INSTALL_PATHS}${JAVA_HOME_16_X64:+$JAVA_HOME_16_X64,}" +JAVA_INSTALL_PATHS="${JAVA_INSTALL_PATHS}${JAVA_HOME_17_X64:+$JAVA_HOME_17_X64,}" +JAVA_INSTALL_PATHS="${JAVA_INSTALL_PATHS}${JAVA_HOME_18_X64:+$JAVA_HOME_18_X64,}" +JAVA_INSTALL_PATHS="${JAVA_INSTALL_PATHS}${JAVA_HOME_19_X64:+$JAVA_HOME_19_X64,}" +JAVA_INSTALL_PATHS="${JAVA_INSTALL_PATHS}${JAVA_HOME_20_X64:+$JAVA_HOME_20_X64,}" +JAVA_INSTALL_PATHS="${JAVA_INSTALL_PATHS}${JAVA_HOME_21_X64:+$JAVA_HOME_21_X64,}" +JAVA_INSTALL_PATHS="${JAVA_INSTALL_PATHS}${JAVA_HOME_22_X64:+$JAVA_HOME_22_X64,}" +JAVA_INSTALL_PATHS="${JAVA_INSTALL_PATHS}${JAVA_HOME_23_X64:+$JAVA_HOME_23_X64,}" + +# Our CI JDKs should be pre-provisioned and invoked correctly, +# we shouldn't rely on gradle for any of this logic. +cat << EOF +org.gradle.java.installations.auto-download=false +org.gradle.java.installations.auto-detect=false +org.gradle.workers.max=${MAX_WORKERS} +org.gradle.java.installations.paths=${JAVA_INSTALL_PATHS} +EOF diff --git a/.github/workflows/build-ci.yml b/.github/workflows/build-ci.yml index 67768721d2e..cbc9a2deafd 100644 --- a/.github/workflows/build-ci.yml +++ b/.github/workflows/build-ci.yml @@ -30,9 +30,7 @@ jobs: - name: Setup gradle properties run: | - cat .github/env/${{ runner.os }}/gradle.properties >> gradle.properties - echo >> gradle.properties - echo "org.gradle.java.installations.paths=${{ steps.setup-java-11.outputs.path }},${{ steps.setup-java-17.outputs.path }}," >> gradle.properties + .github/scripts/gradle-properties.sh >> gradle.properties cat gradle.properties - name: Get Semver @@ -161,9 +159,7 @@ jobs: - name: Setup gradle properties run: | - cat .github/env/${{ runner.os }}/gradle.properties >> gradle.properties - echo >> gradle.properties - echo "org.gradle.java.installations.paths=${{ steps.setup-java-11.outputs.path }}" >> gradle.properties + .github/scripts/gradle-properties.sh >> gradle.properties cat gradle.properties - name: Set up Docker Buildx diff --git a/.github/workflows/check-ci.yml b/.github/workflows/check-ci.yml index 1a1e09d5a9e..f553d7870a6 100644 --- a/.github/workflows/check-ci.yml +++ b/.github/workflows/check-ci.yml @@ -32,9 +32,7 @@ jobs: - name: Setup gradle properties run: | - cat .github/env/${{ runner.os }}/gradle.properties >> gradle.properties - echo >> gradle.properties - echo "org.gradle.java.installations.paths=${{ steps.setup-java-11.outputs.path }},${{ steps.setup-java-17.outputs.path }}," >> gradle.properties + .github/scripts/gradle-properties.sh >> gradle.properties cat gradle.properties - name: Check diff --git a/.github/workflows/docs-ci.yml b/.github/workflows/docs-ci.yml index 3e93c1d5e5e..c4408e4d5d5 100644 --- a/.github/workflows/docs-ci.yml +++ b/.github/workflows/docs-ci.yml @@ -32,9 +32,7 @@ jobs: - name: Setup gradle properties run: | - cat .github/env/${{ runner.os }}/gradle.properties >> gradle.properties - echo >> gradle.properties - echo "org.gradle.java.installations.paths=${{ steps.setup-java-11.outputs.path }},${{ steps.setup-java-17.outputs.path }}" >> gradle.properties + .github/scripts/gradle-properties.sh >> gradle.properties cat gradle.properties - name: All Javadoc @@ -88,9 +86,7 @@ jobs: - name: Setup gradle properties run: | - cat .github/env/${{ runner.os }}/gradle.properties >> gradle.properties - echo >> gradle.properties - echo "org.gradle.java.installations.paths=${{ steps.setup-java-11.outputs.path }},${{ steps.setup-java-17.outputs.path }}" >> gradle.properties + .github/scripts/gradle-properties.sh >> gradle.properties cat gradle.properties - name: Generate Python Docs @@ -150,9 +146,7 @@ jobs: - name: Setup gradle properties run: | - cat .github/env/${{ runner.os }}/gradle.properties >> gradle.properties - echo >> gradle.properties - echo "org.gradle.java.installations.paths=${{ steps.setup-java-11.outputs.path }}" >> gradle.properties + .github/scripts/gradle-properties.sh >> gradle.properties cat gradle.properties - name: Generate C++ Docs diff --git a/.github/workflows/nightly-check-ci.yml b/.github/workflows/nightly-check-ci.yml index ada41dbe702..b39ec85b3e4 100644 --- a/.github/workflows/nightly-check-ci.yml +++ b/.github/workflows/nightly-check-ci.yml @@ -48,9 +48,7 @@ jobs: - name: Setup gradle properties run: | - cat .github/env/${{ runner.os }}/gradle.properties >> gradle.properties - echo >> gradle.properties - echo "org.gradle.java.installations.paths=${{ steps.setup-java-11.outputs.path }},${{ steps.setup-java-17.outputs.path }},${{ steps.setup-java-18.outputs.path }}," >> gradle.properties + .github/scripts/gradle-properties.sh >> gradle.properties cat gradle.properties - name: Run gradle ${{ matrix.gradle-task }} on java ${{ matrix.test-jvm-version }} diff --git a/.github/workflows/nightly-image-check.yml b/.github/workflows/nightly-image-check.yml index d803daccada..ab09855a5e4 100644 --- a/.github/workflows/nightly-image-check.yml +++ b/.github/workflows/nightly-image-check.yml @@ -24,9 +24,7 @@ jobs: - name: Setup gradle properties run: | - cat .github/env/${{ runner.os }}/gradle.properties >> gradle.properties - echo >> gradle.properties - echo "org.gradle.java.installations.paths=${{ steps.setup-java-11.outputs.path }}" >> gradle.properties + .github/scripts/gradle-properties.sh >> gradle.properties cat gradle.properties - name: Run gradle diff --git a/.github/workflows/publish-ci.yml b/.github/workflows/publish-ci.yml index 4cbb90f7094..47e5eeadd53 100644 --- a/.github/workflows/publish-ci.yml +++ b/.github/workflows/publish-ci.yml @@ -32,9 +32,7 @@ jobs: - name: Setup gradle properties run: | - cat .github/env/${{ runner.os }}/gradle.properties >> gradle.properties - echo >> gradle.properties - echo "org.gradle.java.installations.paths=${{ steps.setup-java-11.outputs.path }},${{ steps.setup-java-17.outputs.path }}," >> gradle.properties + .github/scripts/gradle-properties.sh >> gradle.properties cat gradle.properties # TODO(deephaven-core#2614): Improve gradle/CI assemble and publishing of artifacts @@ -52,8 +50,7 @@ jobs: uses: burrunan/gradle-cache-action@v1 with: job-id: publish - # Note: even though we specify org.gradle.parallel=false in our CI gradle.properties, we want to be explicit - # here about no parallelism to ensure we don't create disjointed staging repositories. + # We need to be explicit here about no parallelism to ensure we don't create disjointed staging repositories. arguments: --no-parallel server-netty-app:build server-jetty-app:build py-server:build py-embedded-server:build publish gradle-version: wrapper env: diff --git a/.github/workflows/quick-ci.yml b/.github/workflows/quick-ci.yml index 56dce44d92a..9f5872a6426 100644 --- a/.github/workflows/quick-ci.yml +++ b/.github/workflows/quick-ci.yml @@ -28,9 +28,7 @@ jobs: - name: Setup gradle properties run: | - cat .github/env/${{ runner.os }}/gradle.properties >> gradle.properties - echo >> gradle.properties - echo "org.gradle.java.installations.paths=${{ steps.setup-java-11.outputs.path }}" >> gradle.properties + .github/scripts/gradle-properties.sh >> gradle.properties cat gradle.properties - name: Quick Task diff --git a/.github/workflows/tag-base-images.yml b/.github/workflows/tag-base-images.yml index f38a7d0e5d2..bf4368d01c0 100644 --- a/.github/workflows/tag-base-images.yml +++ b/.github/workflows/tag-base-images.yml @@ -34,9 +34,7 @@ jobs: - name: Setup gradle properties run: | - cat .github/env/${{ runner.os }}/gradle.properties >> gradle.properties - echo >> gradle.properties - echo "org.gradle.java.installations.paths=${{ steps.setup-java-11.outputs.path }},${{ steps.setup-java-17.outputs.path }}," >> gradle.properties + .github/scripts/gradle-properties.sh >> gradle.properties cat gradle.properties - name: Login to ghcr.io diff --git a/buildSrc/src/main/groovy/TestTools.groovy b/buildSrc/src/main/groovy/TestTools.groovy index 9238b9f2bd8..a621c2a0884 100644 --- a/buildSrc/src/main/groovy/TestTools.groovy +++ b/buildSrc/src/main/groovy/TestTools.groovy @@ -1,6 +1,4 @@ -import org.gradle.api.JavaVersion import org.gradle.api.Project -import org.gradle.api.Task import org.gradle.api.artifacts.Dependency import org.gradle.api.plugins.JavaPluginConvention import org.gradle.api.reporting.Report @@ -26,37 +24,24 @@ class TestTools { static final String TEST_GROUP = "~Deephaven Test" - static boolean allowParallel(Project project) { - return Boolean.parseBoolean(project.findProperty('allowParallel') as String ?: 'false') - } - static Test addEngineSerialTest(Project project) { - def allowParallel = allowParallel(project) - // testSerial: non-parallel, not a engine test, isolated - return TestTools.addEngineTest(project, 'Serial', allowParallel, false, true) + return addEngineTest(project, 'Serial', false, true) } static Test addEngineParallelTest(Project project) { - def allowParallel = allowParallel(project) // Add @Category(ParallelTest.class) to have your tests run in parallel // Note: Supports JUnit4 or greater only (you use @Test annotations to mark test methods). - - // The Parallel tests are now by default functionally equivalent to the Serial test logic - // TODO (deephaven-core#643): Fix "leaking" parallel tests - return TestTools.addEngineTest(project, 'Parallel', allowParallel, false, !allowParallel) + return addEngineTest(project, 'Parallel', true, false) } static Test addEngineOutOfBandTest(Project project) { - def allowParallel = allowParallel(project) - // testOutOfBand: non-parallel, not a engine test, not isolated - return TestTools.addEngineTest(project, 'OutOfBand', allowParallel, false, false) + return addEngineTest(project, 'OutOfBand', true, false) } - static Test addEngineTest( + private static Test addEngineTest( Project project, String type, boolean parallel = false, - boolean passwordEnabled = false, boolean isolated = false ) { Test mainTest = project.tasks.getByName('test') as Test @@ -84,11 +69,10 @@ By default only runs in CI; to run locally: dependsOn project.tasks.findByName('testClasses') if (parallel) { - if (project.hasProperty('maxParallelForks')) { - maxParallelForks = project.property('maxParallelForks') as int - } else { - maxParallelForks = 12 - } + // We essentially want to set maxParallelForks for all "normal" tests. It will be capped by the number + // of gradle workers, org.gradle.workers.max. We aren't able to set set this property for all Test types, + // because testSerial needs special handling. + maxParallelForks = Runtime.runtime.availableProcessors() if (project.hasProperty('forkEvery')) { forkEvery = project.property('forkEvery') as int } @@ -124,24 +108,6 @@ By default only runs in CI; to run locally: SourceSetContainer sources = project.convention.getPlugin(JavaPluginConvention).sourceSets setClasspath project.files(sources.getByName('test').output, sources.getByName('main').output, project.configurations.getByName(TEST_RUNTIME_CLASSPATH_CONFIGURATION_NAME)) - if (passwordEnabled) { - def host = project.findProperty("${type.toLowerCase()}.test.host") ?: '0.0.0.0' - def password = project.findProperty("${type.toLowerCase()}.test.password") - t.systemProperty("${type.toLowerCase()}.test.host", host) - t.systemProperty("${type.toLowerCase()}.test.password", password) - - if (password) { - // When running with a valid password, make sure task `check` (and, by dependence, `build`) depends on us. - project.tasks.getByName('check').dependsOn(t) - } else { - // no password? skip these tasks. - t.onlyIf { false } - } - // This is necessary for the mysql/sqlserver tests, which must all be run in a suite. - // Do not cargo-cult this when extending @Category support for other test types. - t.include("**/*Suite.class") - } - // we also need to adjust the reporting output directory of the alt task, // so we don't stomp over top of previous reports. reports.all { diff --git a/buildSrc/src/main/groovy/io.deephaven.java-test-conventions.gradle b/buildSrc/src/main/groovy/io.deephaven.java-test-conventions.gradle index cefbd84752f..e47124696a0 100644 --- a/buildSrc/src/main/groovy/io.deephaven.java-test-conventions.gradle +++ b/buildSrc/src/main/groovy/io.deephaven.java-test-conventions.gradle @@ -23,7 +23,16 @@ artifacts { archives testJar } -project.tasks.withType(Test).all { Test t -> +// This only applies to the standard "test" task +project.tasks.named('test', Test, { test -> + // We essentially want to set maxParallelForks for all "normal" tests. It will be capped by the number + // of gradle workers, org.gradle.workers.max. We aren't able to set set this property for all Test types, + // because testSerial needs special handling. + test.maxParallelForks = Runtime.runtime.availableProcessors() +}) + +// This applies to all test types, including the testOutOfBand, testSerial, and testParallel +project.tasks.withType(Test).configureEach { Test t -> t.with { t.defaultCharacterEncoding = 'UTF-8' @@ -45,10 +54,6 @@ project.tasks.withType(Test).all { Test t -> maxHeapSize = '3g' } - if (!maxParallelForks) { - maxParallelForks = 4 - } - if (findProperty('shortTests') == 'true') { systemProperty 'TstUtils.shortTests', 'true' } diff --git a/engine/table/build.gradle b/engine/table/build.gradle index cedb506a27d..2af2095865d 100644 --- a/engine/table/build.gradle +++ b/engine/table/build.gradle @@ -128,8 +128,6 @@ spotless { } test { - maxParallelForks = Integer.parseInt(findProperty('maxParallelForks') as String ?: '1') - forkEvery = Integer.parseInt(findProperty('forkEvery') as String ?: '1') // For now, if you apply @Category(ParallelTest.class) to tests which are not huge CPU/RAM hogs, you can get parallelism // If you have CPU/RAM-heavy tasks that you don't want gumming up :engine-table:test runs, apply @Category(SerialTest.class) instead // (note that the above only works for junit 4 tests; see the documentation on SerialTest class and others for porting instructions) @@ -147,6 +145,7 @@ TestTools.addEngineParallelTest(project) TestTools.addEngineSerialTest(project) TestTools.addEngineOutOfBandTest(project) +// If changing, be sure to update .github/scripts/print-gradle-workers-max.sh def maxHeapSize = findProperty('maxHeapSize') as String ?: '6g' tasks.testParallel.maxHeapSize = maxHeapSize diff --git a/gradle.properties b/gradle.properties index 5d21fc36494..5346ddb25c8 100644 --- a/gradle.properties +++ b/gradle.properties @@ -15,6 +15,13 @@ # The gradle workers often have their own methods to control additional heap. # For example, compilation and java tests are able to set their own memory usage needs through their own configurations. org.gradle.jvmargs=-Xmx1g +org.gradle.parallel=true + +# If you are running the full test suite locally, or running gradle builds on infrastructure that has a lot of CPUs but +# is memory-limited, you may need to explicitly tune your max workers. +# See .github/scripts/gradle-properties.sh for the logic we use there. +# See https://github.com/gradle/gradle/issues/14431#issuecomment-1601724453. +#org.gradle.workers.max=... # Setting debugCI to true will cause failed builds to keepalive for three hours so we can do post-mortem inspection of jenkins workspace debugCI=false