feat: enable Groovy invokedynamic by default on Grails 9 - #16165
Open
jamesfredley wants to merge 25 commits into
Open
feat: enable Groovy invokedynamic by default on Grails 9#16165jamesfredley wants to merge 25 commits into
jamesfredley wants to merge 25 commits into
Conversation
Grails 7 turned indy off because Groovy 4 paid a 2-8x penalty (#15293). Grails 9 is on Groovy 6, where microbenchmarks were already positive and app-level results were flat to slightly negative. Make indy the default so generated apps match Groovy 6's preferred dispatch. Opt out with grails { indy = false }. Non-indy still needs groovy-callsite, which the BOM already manages. Framework modules honor -PgrailsIndy so CI and JMH can still A/B both modes. Assisted-by: claude-code:claude-opus-5
Contributor
There was a problem hiding this comment.
Pull request overview
This PR flips the default Groovy invokedynamic (“indy”) setting to enabled for Grails 9 builds, aligning generated applications with Groovy 6’s preferred dynamic dispatch mode while preserving an opt-out path and CI/framework-module toggling.
Changes:
- Set the
grails { indy }convention default totrueand update plugin logging accordingly. - Add build-logic support for
-PgrailsIndyto control indy in framework modules, including addingorg.apache.groovy:groovy-callsitewhen indy is forced off. - Add/extend tests and documentation covering the new default and the opt-out configuration.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| grails-gradle/plugins/src/test/groovy/org/grails/gradle/plugin/core/GrailsExtensionSpec.groovy | Adds tests asserting indy default-on and opt-out behavior in the Gradle extension. |
| grails-gradle/plugins/src/main/groovy/org/grails/gradle/plugin/core/GrailsGradlePlugin.groovy | Changes indy default handling to enabled and updates informational logging. |
| grails-gradle/plugins/src/main/groovy/org/grails/gradle/plugin/core/GrailsExtension.groovy | Sets the indy convention default to true and updates associated Javadoc. |
| grails-doc/src/en/guide/introduction/whatsNew.adoc | Documents indy being enabled by default and how to disable it. |
| build-logic/plugins/src/test/groovy/org/apache/grails/buildsrc/CompilePluginIndySpec.groovy | Adds coverage for -PgrailsIndy behavior in framework-module compilation configuration. |
| build-logic/plugins/src/main/groovy/org/apache/grails/buildsrc/CompilePlugin.groovy | Honors -PgrailsIndy for framework modules and adds groovy-callsite when indy is disabled. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
JMH Shard PairingComplete shard pairs used: 2 JMH Benchmark ReportRegressions: 0 Group geometric means are descriptive only, not verdicts.
Per-benchmark results
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 9.0.x #16165 +/- ##
==================================================
+ Coverage 53.1116% 53.1171% +0.0055%
+ Complexity 19362 19340 -22
==================================================
Files 2081 2081
Lines 99018 99019 +1
Branches 17375 17390 +15
==================================================
+ Hits 52590 52596 +6
- Misses 38867 38880 +13
+ Partials 7561 7543 -18
🚀 New features to boost your workflow:
|
-PgrailsIndy=false is how CI compiles applications without indy. Wiring
that property through CompilePlugin forced every framework module onto
classic callsites and added org.apache.groovy:groovy-callsite with no
version, so those jobs failed with "Could not find groovy-callsite:.".
Leave framework compilation on Groovy's default. App indy remains
controlled by grails { indy }. Clarify the docs so default-on does not
read like a required setting.
Assisted-by: claude-code:claude-opus-5
Contributor
Indy Whiplash?I agree with this change, but I wonder if we should consider turning indy on in the final release of 8? I think it is counter intuitive to go against Groovy defaults and turn it opposite to normal compiler behavior. At the very least we should consider not changing downstream plugin behavior and perhaps enforce plugins indy true? The following should be fully understood prior to 8.0.0:
Current state of Grails 8 indy OFF (29): indy ON (17, Groovy 5 default): |
MiscSpec is identical to 8.0.x and passed on 9.0.x when the plugin defaulted to indy=false. With indy on by default, Map-as-FilterChain and session[name] are fragile Groovy dispatch. Use a real FilterChain and HttpSession.getAttribute instead. Assisted-by: claude-code:claude-opus-5
Contributor
Author
|
@codeconsole only holdup is Groovy 5 indy is not as fast as Groovy 6 indy. |
MiscSpec failed only when the security plugin compiled with indy on.
The session dump already contained Username=admin. with(auth) {
contains(...) } does not dispatch to String.contains under Groovy 6
indy. Direct pageSource.contains already passed. Call contains on the
string instead and snapshot pageSource once per assertion block.
Assisted-by: claude-code:claude-opus-5
The same dispatch bug as MiscSpec. ACL and UI functional specs call
with(pageSource) { contains(...) }, which does not hit String.contains
when compiled with invokedynamic. Use pageSource.contains instead.
Assisted-by: claude-code:claude-opus-5
org.gradle.jvmargs sizes the Gradle daemon only. Test forks are separate child JVMs that take their heap from maxHeapSize in gradle/test-config.gradle, so a job's configured heap is the daemon -Xmx plus the concurrent test forks times the per-fork heap. Those two numbers live in different files and have never been reasoned about together. The concurrent fork count is not maxParallelForks. With org.gradle.parallel=true several Test tasks run at once, so the live JVM count is bounded by Gradle's global worker pool, which defaults to the CPU count. On the 4-CPU, ~16 GB Linux and Windows runners that floor is 5G + 4x768m = 8G and fits. On the 3-CPU, ~7 GB macOS runner it is 5G + 3x768m = 7.25G and does not. Cap --max-workers on the macOS leg, since that is what actually limits concurrent test and compiler JVMs, and keep maxTestParallel alongside it so no single task exceeds the same cap. Both are passed through a new runner_arguments matrix key that is undefined, and therefore empty, for every other entry. The daemon stays at 5 GB: groovydoc is what needs it, and shrinking it would trade a memory problem for a slower build. Document the arithmetic next to org.gradle.jvmargs as a simplified configured-heap floor, explicitly excluding metaspace, native memory and the forked compiler workers that CompilePlugin gives their own -Xmx2G, so it is not mistaken for a true peak. This changes concurrency only. No test is added, removed, skipped or weakened. Assisted-by: claude-code:claude-opus-5
UserSimpleSpec failed because with(page.rolesTab) { hasEnabledRole(...) }
dispatched to Geb DefaultNavigator under Groovy 6 invokedynamic. Call
the tab methods on page.rolesTab instead.
Assisted-by: claude-code:claude-opus-5
Contributor
Author
App-level indy vs callsite (local A/B)Ran the existing Machine: Windows, JDK 21, warmup 80 / samples 300 / forks 2.
This matches the earlier story: app-level is flat to slightly negative, not the old 2-8x Groovy 4 penalty. A follow-up |
3 tasks
jdaugherty
reviewed
Aug 19, 2026
| contains('93') | ||
| contains('95') | ||
| } | ||
| pageSource.contains('75') |
Contributor
There was a problem hiding this comment.
Why are we removing with { } in these assertion blocks? This is a supported feature of spock that @matrei had added: https://spockframework.org/spock/docs/2.5-SNAPSHOT/all_in_one.html#_using_with_for_expectations
jdaugherty
reviewed
Aug 19, 2026
jdaugherty
left a comment
jdaugherty
left a comment
Contributor
There was a problem hiding this comment.
let's remove teh mass test changes that removed the with usage in tests?
Groovy 5 defaults indy on. Only modules that apply the Grails Gradle plugin inherited grails.indy=false, so published framework artifacts were mixed. Centralize indy=false in CompilePlugin and apply gradle/groovy-indy.gradle from the grails-core, grails-gradle, and grails-forge builds. CI can still opt in with -PgrailsIndy=true. See #15293 Assisted-by: Sisyphus:grok-4.6
With indy off, log.debug inside GormStaticApi.count()'s session callback was dispatched as Domain.debug(...). Capture the @slf4j logger in a local first. Also trim -PgrailsIndy the same way as CompilePlugin and apply the shared groovy-indy script from build-logic. Assisted-by: Sisyphus:grok-4.6
Class.newInstance(Map) is not selected under @CompileStatic when invokedynamic is disabled. Use InvokerHelper.invokeConstructorOf with an explicit Object[] so nested Map-constructor types still bind. Also avoid `null as boolean` in the Map-constructor test fixture, which Groovy 5 throws on without indy after unbindable properties are filtered from constructor arguments. Assisted-by: Sisyphus:grok-4.6
Honor review: drop groovy-indy.gradle and subprojects applies. Unpublished build-logic uses Gradle's Groovy default.
Contributor
Author
Hibernate 7 publishes jboss-logging as a runtime-only transitive. Groovydoc Class.forName's referenced types, so :grails-data-hibernate7-dbmigration-core:groovydoc failed in CI (Forge/e2e publish) with NoClassDefFoundError: org/jboss/logging/Logger.
build(ci): bound concurrent JVMs on the macOS runner
build: disable Groovy invokedynamic for the Grails 8 compile
Take #16178's CompilePlugin -PgrailsIndy toggle and flip the default to true so Grails 9 / Groovy 6 framework modules match app-level grails { indy } (already default-on). CI can still pass -PgrailsIndy=false. Assisted-by: Sisyphus:grok-4.6
Contributor
Author
|
Brought in #16178 via the 8.0.x -> 8.1.x -> 9.0.x merge-up (#16318, #16319).
|
🚨 TestLens detected 8 failed tests 🚨Here is what you can do:
Test SummaryCI / Build Gradle Plugins (21) > :grails-gradle-plugins:test [grails-gradle]
CI / Build Gradle Plugins (25) > :grails-gradle-plugins:test [grails-gradle]
CI / Build Grails-Core (Ubuntu JDK 21) > :grails-taglib:test
CI / Build Grails-Core (Ubuntu JDK 25) > :grails-taglib:test
CI / Build Grails-Core (Windows JDK 25 shard 2) > :grails-taglib:test
CI / Build Grails-Core (macOS JDK 21) > :grails-taglib:test
CI / Build Grails-Core Rerunning all Tasks (Ubuntu JDK 21 shard 2) > :grails-taglib:test
SiteMesh 2 Compatibility / SiteMesh 2 Core Tests (Java 21) > :grails-taglib:test
🏷️ Commit: 919a12c Test FailuresGrailsNativeImageDefaultsSpec > an application that builds no image is left as it was (:grails-gradle-plugins:test [grails-gradle] in CI / Build Gradle Plugins (21))GrailsNativeImageDefaultsSpec > an application that builds no image is left as it was (:grails-gradle-plugins:test [grails-gradle] in CI / Build Gradle Plugins (25))GroovyPageAttributesTests > testAssigningGspTagSyntaxCallInvokesTheSetter() (:grails-taglib:test in CI / Build Grails-Core (Ubuntu JDK 21))
GroovyPageAttributesTests > testAssigningGspTagSyntaxCallInvokesTheSetter() (:grails-taglib:test in CI / Build Grails-Core (Ubuntu JDK 25))
GroovyPageAttributesTests > testAssigningGspTagSyntaxCallInvokesTheSetter() (:grails-taglib:test in CI / Build Grails-Core (Windows JDK 25 shard 2))
GroovyPageAttributesTests > testAssigningGspTagSyntaxCallInvokesTheSetter() (:grails-taglib:test in CI / Build Grails-Core (macOS JDK 21))
GroovyPageAttributesTests > testAssigningGspTagSyntaxCallInvokesTheSetter() (:grails-taglib:test in CI / Build Grails-Core Rerunning all Tasks (Ubuntu JDK 21 shard 2))
GroovyPageAttributesTests > testAssigningGspTagSyntaxCallInvokesTheSetter() (:grails-taglib:test in SiteMesh 2 Compatibility / SiteMesh 2 Core Tests (Java 21))
Rerun ControlsSelect tests to mute in this pull request:
Reuse successful test results:
Click the checkbox to trigger a rerun:
Learn more about TestLens at testlens.app/docs. |
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
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.
Purpose
Enable Groovy invokedynamic by default on Grails 9.
Grails 7 disabled indy because Groovy 4 paid a 2-8x penalty (#15293). Grails 9 is on Groovy 6. Earlier A/B work on
perf/indy-vs-callsite-g6showed:That is still a large improvement over the old 2-8x hit. This PR flips the default so generated apps and published framework modules match Groovy 6's preferred dispatch. The
performancelabel is set so CI JMH can measure this default against9.0.x.What changed
grails { indy }convention is nowtruegrails { indy = false }CompilePluginuses the build: disable Groovy invokedynamic for the Grails 8 compile #16178-PgrailsIndytoggle, default true on Grails 9 (Grails 8 stays false)groovy-callsite(BOM-managed)whatsNew.adocdocuments the default and the opt-outNot in this PR
The full app-bench harness from
perf/indy-vs-callsite-g6is not copied here. That branch is 3.7k lines and stale vs current9.0.x. CI JMH via theperformancelabel is the measurement path.Test plan
GrailsExtensionSpecdefault-on and opt-outCompilePluginSpecdefault true /-PgrailsIndy=false/-PgrailsIndy=trueperformanceJMH vs9.0.x