Copilot commented on code in PR #15375:
URL: https://github.com/apache/grails-core/pull/15375#discussion_r2827986270
##########
grails-gradle/plugins/src/main/groovy/org/grails/gradle/plugin/core/GrailsGradlePlugin.groovy:
##########
@@ -184,9 +184,14 @@ class GrailsGradlePlugin extends GroovyPlugin {
private void configureGroovyCompiler(Project project) {
Provider<RegularFile> groovyCompilerConfigFile =
project.layout.buildDirectory.file('grailsGroovyCompilerConfig.groovy')
+ GrailsExtension grailsExtension =
project.extensions.findByType(GrailsExtension)
+
project.tasks.withType(GroovyCompile).configureEach { GroovyCompile c
->
c.outputs.file(groovyCompilerConfigFile)
+ // Configure indy option from GrailsExtension
+ c.groovyOptions.optimizationOptions.indy =
grailsExtension?.indy?.getOrElse(false) ?: false
+
Review Comment:
`GroovyCompile` tasks read `grailsExtension.indy` during plugin `apply`,
which occurs before the build script typically configures the `grails { ... }`
extension. As a result, `grails { indy = true }` may not affect already-created
`GroovyCompile` tasks, leading to indy staying disabled even when the extension
is enabled. Configure the `GroovyCompile` indy flag after project evaluation
(or otherwise ensure it reflects the final extension value) so user
configuration reliably takes effect.
##########
grails-doc/src/en/guide/upgrading/upgrading60x.adoc:
##########
@@ -61,16 +61,27 @@ In your gradle file, you can force a dependency upgrade via
this code:
}
----
-* By default, Groovy 4 switches away from callsite optimizations and uses
invokedynamic instead. This can result in performance regressions compared to
Grails 6. Groovy 5 will remove the ability to disable invokedynamic, but to
disable it for Groovy 4, modify your `build.gradle` to include the following:
+* By default, Groovy 4 switches away from callsite optimizations and uses
invokedynamic instead. This can result in performance regressions compared to
Grails 6. The Grails Gradle Plugin now automatically disables invokedynamic for
all `GroovyCompile` tasks (see
https://github.com/apache/grails-core/issues/15293[#15293]). If you have a
manual `tasks.withType(GroovyCompile)` block in your `build.gradle` that sets
`indy = false`, you can safely remove it:
[source,groovy]
-.build.gradle
+.build.gradle - remove this block (no longer needed)
----
+ // This is now handled by the Grails Gradle Plugin - remove it
tasks.withType(GroovyCompile).configureEach {
groovyOptions.optimizationOptions.indy = false
}
----
+To re-enable invokedynamic, use the `grails` extension in your `build.gradle`:
+
+[source,groovy]
+.build.gradle
+----
+ grails {
+ indy = true
Review Comment:
This snippet instructs users to enable indy via `grails { indy = true }`,
but the extension currently exposes `indy` as a `Property<Boolean>`. Unless the
extension provides a dedicated setter that maps assignment to `indy.set(true)`,
this syntax may not work in Groovy DSL as written. Align the documentation with
the supported configuration syntax (or adjust the extension API to support
assignment).
```suggestion
indy.set(true)
```
##########
grails-gradle/plugins/src/main/groovy/org/grails/gradle/plugin/core/GrailsExtension.groovy:
##########
@@ -76,6 +78,14 @@ class GrailsExtension {
*/
boolean micronautAutoSetup = true
+ /**
+ * Whether to enable Groovy's invokedynamic (indy) bytecode instruction
for dynamic Groovy method dispatch.
+ * Disabled by default to improve performance (see GitHub issue #15293).
+ * When enabled, Groovy uses JVM invokedynamic instead of traditional
callsite caching.
+ * To enable invokedynamic in build.gradle: grails { indy = true }
+ */
+ final Property<Boolean> indy
+
Review Comment:
The user-facing enablement instructions reference `grails { indy = true }`,
but `indy` is declared as `Property<Boolean>`. Without an explicit setter
(e.g., `setIndy(boolean)` delegating to `indy.set(...)`), Groovy DSL assignment
may not work as documented. Either add a setter to support `indy = true` or
update the message/docs to use `indy.set(true)` (and keep the DSL consistent
with how you want users to configure it).
```suggestion
/**
* Allows Groovy DSL-style configuration such as {@code grails { indy =
true }} to work
* by delegating to the underlying {@link Property} instance.
*
* @param enabled whether invokedynamic should be enabled
*/
void setIndy(boolean enabled) {
this.indy.set(enabled)
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
