From 1e37b0873b06a37b6998420751c7b8082276b256 Mon Sep 17 00:00:00 2001 From: "Flavio S. Glock" Date: Sun, 26 Apr 2026 17:34:12 +0200 Subject: [PATCH] test: balance unit test shards by isolating heavy test Two related changes to the parallel unit test sharding: 1. Add a 5th shard dedicated to known-heavy tests (currently just unit/code_too_large.t at ~16s). Other shards round-robin the remaining tests. Heavy tests are listed in HEAVY_TESTS in PerlScriptExecutionTest.java and easy to extend. 2. Fix a pre-existing off-by-one bug in shard index handling. The "Maven surefire is 1-indexed" conversion triggered for any shardIndex in [1, shardTotal], which incorrectly caught Gradle's 0-indexed values. Result: shard 1 ran the same tests as shard 0 and the last shard's tests were never run at all (about 25% of unit tests silently skipped, including code_too_large.t after the 5-shard change). Both backends now pass 0-indexed shard.index values; the conversion is removed. Wall-time improvement on `make`: ~32s -> ~22-26s, while also fixing the silent test-skipping correctness bug. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- Makefile | 10 ++-- build.gradle | 10 +++- pom.xml | 29 ++++++++-- .../org/perlonjava/core/Configuration.java | 6 +- .../perlonjava/PerlScriptExecutionTest.java | 56 +++++++++++++++---- 5 files changed, 83 insertions(+), 28 deletions(-) diff --git a/Makefile b/Makefile index 3ca4a5848..1411dfcfe 100644 --- a/Makefile +++ b/Makefile @@ -31,7 +31,7 @@ check-java-gradle: wrapper: check-java-gradle -# Standard build - incremental compilation with parallel tests (4 JVMs) +# Standard build - incremental compilation with parallel tests (5 JVMs; last shard isolates heavy tests) build: check-java-gradle ifeq ($(OS),Windows_NT) gradlew.bat classes testUnitParallel --parallel shadowJar @@ -149,7 +149,7 @@ else ./gradlew testAll --rerun-tasks endif -# Parallel unit tests via Gradle/JUnit (4 JVMs) +# Parallel unit tests via Gradle/JUnit (5 JVMs; last shard isolates heavy tests) test-gradle-parallel: check-java-gradle ifeq ($(OS),Windows_NT) gradlew.bat testUnitParallel --parallel --rerun-tasks @@ -157,12 +157,12 @@ else ./gradlew testUnitParallel --parallel --rerun-tasks endif -# Parallel unit tests via Maven (4 JVMs) +# Parallel unit tests via Maven (5 JVMs; last shard isolates heavy tests) test-maven-parallel: ifeq ($(OS),Windows_NT) - start /B mvn test -Pshard1 & start /B mvn test -Pshard2 & start /B mvn test -Pshard3 & start /B mvn test -Pshard4 + start /B mvn test -Pshard1 & start /B mvn test -Pshard2 & start /B mvn test -Pshard3 & start /B mvn test -Pshard4 & start /B mvn test -Pshard5 else - mvn test -Pshard1 & mvn test -Pshard2 & mvn test -Pshard3 & mvn test -Pshard4 & wait + mvn test -Pshard1 & mvn test -Pshard2 & mvn test -Pshard3 & mvn test -Pshard4 & mvn test -Pshard5 & wait endif clean: check-java-gradle diff --git a/build.gradle b/build.gradle index 173d96701..9fa121ec9 100644 --- a/build.gradle +++ b/build.gradle @@ -402,7 +402,12 @@ tasks.named('processTestResources', Copy) { // Parallel test execution tasks // Run with: ./gradlew testUnitParallel --parallel -def parallelShards = 4 +// +// Shard layout: the LAST shard is dedicated to a small set of known-heavy +// tests (see HEAVY_TESTS in PerlScriptExecutionTest.java). The other shards +// round-robin the remaining tests. This keeps wall-time roughly balanced +// even when one test dominates (e.g. unit/code_too_large.t). +def parallelShards = 5 (0.. tasks.register("testUnitShard${index}", Test) { @@ -425,7 +430,8 @@ def parallelShards = 4 tasks.register('testUnitParallel') { group = 'verification' description = 'Runs unit tests in parallel across multiple JVMs. Usage: gradle testUnitParallel --parallel' - dependsOn 'testUnitShard0', 'testUnitShard1', 'testUnitShard2', 'testUnitShard3' + def shardTasks = (0.. - @@ -233,7 +236,7 @@ org.apache.maven.plugins maven-surefire-plugin - --enable-native-access=ALL-UNNAMED -Dtest.shard.index=1 -Dtest.shard.total=4 + --enable-native-access=ALL-UNNAMED -Dtest.shard.index=0 -Dtest.shard.total=5 @@ -247,7 +250,7 @@ org.apache.maven.plugins maven-surefire-plugin - --enable-native-access=ALL-UNNAMED -Dtest.shard.index=2 -Dtest.shard.total=4 + --enable-native-access=ALL-UNNAMED -Dtest.shard.index=1 -Dtest.shard.total=5 @@ -261,7 +264,7 @@ org.apache.maven.plugins maven-surefire-plugin - --enable-native-access=ALL-UNNAMED -Dtest.shard.index=3 -Dtest.shard.total=4 + --enable-native-access=ALL-UNNAMED -Dtest.shard.index=2 -Dtest.shard.total=5 @@ -275,7 +278,21 @@ org.apache.maven.plugins maven-surefire-plugin - --enable-native-access=ALL-UNNAMED -Dtest.shard.index=4 -Dtest.shard.total=4 + --enable-native-access=ALL-UNNAMED -Dtest.shard.index=3 -Dtest.shard.total=5 + + + + + + + shard5 + + + + org.apache.maven.plugins + maven-surefire-plugin + + --enable-native-access=ALL-UNNAMED -Dtest.shard.index=4 -Dtest.shard.total=5 diff --git a/src/main/java/org/perlonjava/core/Configuration.java b/src/main/java/org/perlonjava/core/Configuration.java index 5b8f11f02..6df730fd2 100644 --- a/src/main/java/org/perlonjava/core/Configuration.java +++ b/src/main/java/org/perlonjava/core/Configuration.java @@ -33,14 +33,14 @@ public final class Configuration { * Automatically populated by Gradle/Maven during build. * DO NOT EDIT MANUALLY - this value is replaced at build time. */ - public static final String gitCommitId = "0663a8089"; + public static final String gitCommitId = "2c7f37913"; /** * Git commit date of the build (ISO format: YYYY-MM-DD). * Automatically populated by Gradle/Maven during build. * DO NOT EDIT MANUALLY - this value is replaced at build time. */ - public static final String gitCommitDate = "2026-04-25"; + public static final String gitCommitDate = "2026-04-26"; /** * Build timestamp in Perl 5 "Compiled at" format (e.g., "Apr 7 2026 11:20:00"). @@ -48,7 +48,7 @@ public final class Configuration { * Parsed by App::perlbrew and other tools via: perl -V | grep "Compiled at" * DO NOT EDIT MANUALLY - this value is replaced at build time. */ - public static final String buildTimestamp = "Apr 26 2026 09:17:16"; + public static final String buildTimestamp = "Apr 26 2026 17:32:55"; // Prevent instantiation private Configuration() { diff --git a/src/test/java/org/perlonjava/PerlScriptExecutionTest.java b/src/test/java/org/perlonjava/PerlScriptExecutionTest.java index 7e3d6fd82..4477abfe3 100644 --- a/src/test/java/org/perlonjava/PerlScriptExecutionTest.java +++ b/src/test/java/org/perlonjava/PerlScriptExecutionTest.java @@ -122,27 +122,59 @@ private static Stream getPerlScripts(boolean unitOnly) throws IOExceptio return sortedScripts.stream(); } - // Sharding logic + // Sharding logic. + // + // We use a simple "heavy-test isolation" scheme for balance: + // - The LAST shard is dedicated to known-heavy tests (HEAVY_TESTS). + // - All other shards round-robin the remaining (light) tests. + // + // Rationale: a single test (currently unit/code_too_large.t at ~16s) + // can dominate one shard's wall time well beyond what round-robin + // can balance. Putting it on its own shard lets the others split + // the rest evenly. To extend, just add filenames to HEAVY_TESTS. String shardIndexProp = System.getProperty("test.shard.index"); String shardTotalProp = System.getProperty("test.shard.total"); - - if (shardIndexProp != null && !shardIndexProp.isEmpty() && + + if (shardIndexProp != null && !shardIndexProp.isEmpty() && shardTotalProp != null && !shardTotalProp.isEmpty()) { try { int shardIndex = Integer.parseInt(shardIndexProp); int shardTotal = Integer.parseInt(shardTotalProp); - - // Maven surefire.forkNumber is 1-indexed, convert to 0-indexed - if (shardIndex >= 1 && shardIndex <= shardTotal) { - shardIndex = shardIndex - 1; - } - + + // Both Gradle and Maven now pass 0-indexed shard.index values + // (see build.gradle and pom.xml). Earlier code attempted to + // detect Maven 1-indexed values heuristically, which silently + // collapsed shard 1 onto shard 0 and dropped the last shard + // entirely; do not reintroduce that. + if (shardTotal > 1 && shardIndex >= 0 && shardIndex < shardTotal) { System.out.println("Running shard " + (shardIndex + 1) + " of " + shardTotal); + + // Tests known to be much slower than the rest. Use forward + // slashes; we match against both '/' and '\' separators. + final List HEAVY_TESTS = List.of( + "unit/code_too_large.t" + ); + java.util.function.Predicate isHeavy = s -> { + String norm = s.replace('\\', '/'); + return HEAVY_TESTS.contains(norm); + }; + + final int lastShard = shardTotal - 1; + if (shardIndex == lastShard) { + // Dedicated shard: only the heavy tests. + return sortedScripts.stream().filter(isHeavy); + } + + // Light shards: round-robin over the remaining (shardTotal - 1) shards. + List lightScripts = sortedScripts.stream() + .filter(isHeavy.negate()) + .collect(Collectors.toList()); + final int lightShards = shardTotal - 1; final int finalShardIndex = shardIndex; - return IntStream.range(0, sortedScripts.size()) - .filter(i -> i % shardTotal == finalShardIndex) - .mapToObj(sortedScripts::get); + return IntStream.range(0, lightScripts.size()) + .filter(i -> i % lightShards == finalShardIndex) + .mapToObj(lightScripts::get); } } catch (NumberFormatException e) { // Silently fall through to run all tests