feat: enable Groovy invokedynamic by default on Grails 9 - #16165
feat: enable Groovy invokedynamic by default on Grails 9#16165jamesfredley wants to merge 6 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
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
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
|
@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
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
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 |
✅ All tests passed ✅🏷️ Commit: ccce1ba Learn more about TestLens at testlens.app. |
| contains('93') | ||
| contains('95') | ||
| } | ||
| pageSource.contains('75') |
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
left a comment
jdaugherty
left a comment
There was a problem hiding this comment.
let's remove teh mass test changes that removed the with usage in tests?
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 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 }CompilePluginhonors-PgrailsIndyfor framework modules (same toggle CI already uses)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-outCompilePluginIndySpecabsent / true / falseperformanceJMH vs9.0.x