ci: collapse 5 sequential gradle invocations into 1#7575
Merged
Conversation
- NameList/RuleSet: precompute a cumulative-weight array at construction so pick() is O(log n) via binarySearch instead of two linear passes. Zero-weight entries are filtered into a parallel positives array, which keeps binarySearch correct when duplicates would otherwise let it land on a zero-weight index. Both records become final classes since cached state can't live on a record. - DataValue: drop the hand-rolled DataSubValue linked list in favour of a lazily-allocated LinkedHashMap. putIfAbsent preserves the first-write-wins semantics the old recursive get() relied on. - NameGenLazyData.gendersForRuleset: read directly from the ruleset's RuleSetMeta.categoryTitles() instead of scanning every Sex: bucket. - NameGenerator.assemble: defer meaning/pronunciation StringBuilder allocation until a part actually overrides one, seeding from the name built so far. Avoids two builders' worth of churn on the common path where no part has sub-values.
The jlink plugin's `additive = true` mode emits both the auto-derived and the user-listed `requires` blocks without dedup. asm 9.10 (pulled in by jlink 4.0.1) parses Java 25 class files that 9.9.1 silently skipped, growing the auto-derived set enough to collide with our explicit list and breaking `createMergedModule` with duplicate-requires errors. Bump to 4.0.2 and keep only the modules the scanner can't infer on its own.
Brief description of each overload's contract (direct Element children, optional tag-name filter) so future readers don't have to re-derive it from the stream pipeline.
Drop the Os.FAMILY_MAC || Os.FAMILY_UNIX predicate that excluded Windows from fullJpackage. The carve-out dates from when Windows shipped via NSIS (`buildNsis`); that path was retired in be48763 but the release-side hookup to invoke jpackage on Windows was never added, leaving Windows release artifacts as the runtime zip only.
fullJpackage was wired to assembleJpackageImage (Mac: patchMacJpackage), not the jpackage installer task — so each release shipped only the jlinkZip runtime image, never a .dmg/.deb/.exe. Make jpackage depend on the data-bundling step so it packages an image that already contains data/plugins/preview/outputsheets, then point fullJpackage at jpackage. Pin one installerType per OS (dmg / deb / exe) — without it jpackage emits both formats per platform, doubling build time and (on Linux) requiring rpmbuild that isn't installed on the Ubuntu runner. Add --win-shortcut / --win-menu so the .exe creates Start-menu and desktop entries; the existing --linux-shortcut and Mac-specific opts were already in place. WiX 3.14 needed for the .exe is preinstalled on the windows-latest runner.
Same self-contained app the installer ships, but as an unzip-and-run archive — useful for users who don't want to run an installer (USB sticks, locked-down environments, etc.). Per-platform: pcgen-<version>-<os>-<arch>-portable.zip. The zip captures whatever fullJpackage left in build/jpackage/PcGen[.app], so it includes data/plugins/preview/outputsheets and (on Mac) the patched Info.plist + MacDirLauncher.
jpackage names Debian packages pcgen_<version>_<arch>.deb (underscore), which the existing pcgen-*.* glob (hyphen) misses. Linux release runs were uploading everything except the .deb. Add a second include for the underscore form. .rpm uses pcgen-<version>.<arch>.rpm so it was already covered.
Contributor
Author
The image-*.zip artifacts (output of jlinkZip) are a JVM runtime image, not a runnable PcGen — no launcher, no /data, /plugins, /preview, or /outputsheets. Users who tried to run them got the "missing required folders" dialog. Now that fullJpackage produces real installers and a portable zip with the same .app the installer ships, image-*.zip is just a confusing leftover. Drop it from the release upload (autobuild keeps it via buildDist). The SHA256-digests-*.txt files are also redundant: GitHub computes a SHA-256 for every release asset automatically and exposes it via the API. Anyone verifying integrity can shasum the file locally.
Each ./gradlew invocation pays ~5-10s of configuration overhead and
~2-3s of daemon startup, plus a fresh task-graph build. The release
workflows ran 5 of them in series:
./gradlew clean build # `clean` does nothing on a fresh runner
./gradlew compileSlowtest datatest pfinttest
./gradlew allReports # = checkstyleMain + pmdMain + spotbugsMain,
# all already executed by `build`
./gradlew buildDist # already a transitive dep of pcgenRelease
./gradlew pcgenRelease
Collapse to one invocation, drop the redundant `clean` and `allReports`,
and drop `buildDist` (which pcgenRelease already depends on transitively
via assembleArtifacts → jlinkZip).
Also set fail-fast: false on the matrix so a transient runner preemption
(seen on the partner-supplied ubuntu-24.04-arm image) doesn't kill the
other three OS jobs.
Vest
force-pushed
the
ci_optimize
branch
2 times, most recently
from
3661257 to
03fcb9f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Stacks on top of #7568 — please review and merge that PR first.
Each
./gradlewinvocation pays ~5–10s of configuration overhead and ~2–3s of daemon startup, plus a fresh task-graph build. The release workflows ran 5 of them in series:./gradlew clean buildcleandoes nothing on a fresh CI runner —build/doesn't exist yet./gradlew compileSlowtest datatest pfinttest./gradlew allReports= checkstyleMain + pmdMain + spotbugsMain, all already executed bybuild(verified in the previous run's log:> Task :spotbugsMainran duringbuild, then again asUP-TO-DATEhere)./gradlew buildDistpcgenRelease./gradlew pcgenReleaseCollapse to a single invocation, drop the redundant
cleanandallReports, and dropbuildDist. Also setfail-fast: falseon the matrix so a transient runner preemption (seen on the partner-suppliedubuntu-24.04-armimage — see #7568 verification run) doesn't kill the other three OS jobs.Measured impact
Identical input (same
fix_packagingHEAD with PR #7568 applied), back-to-back runs onVest/pcgenwith warm caches:Windows benefited the most because each
gradlewstartup is dominated by JVM warmup + slower disk I/O on that runner image.Test plan
26779067448)..dmg,.exe), portable zips, jlinkZip images, sources jar, SHA256 manifests.--continueflag).