[ 
https://issues.apache.org/jira/browse/GROOVY-10307?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18063211#comment-18063211
 ] 

ASF GitHub Bot commented on GROOVY-10307:
-----------------------------------------

Copilot commented on code in PR #2393:
URL: https://github.com/apache/groovy/pull/2393#discussion_r2889800759


##########
build-logic/src/main/groovy/org.apache.groovy-performance.gradle:
##########
@@ -65,8 +65,12 @@ tasks.named('jmhClasses') {
     dependsOn tasks.named('clean')
 }
 
+var indy = (project.findProperty('indy') ?: 'true').toBoolean()
+
 tasks.named('compileJmhGroovy') {
     classpath += compileJmhJava.classpath
+    groovyOptions.optimizationOptions.indy = indy
+    inputs.property('indy', indy)

Review Comment:
   `var indy = ...` is Kotlin/Java syntax and isn’t valid in a Groovy Gradle 
script (`.gradle`). This will fail the build during configuration. Declare it 
using Groovy syntax (e.g., `def`/`boolean`) and keep the value derived from the 
`-Pindy` project property.



##########
.github/workflows/groovy-jmh.yml:
##########
@@ -0,0 +1,59 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+name: jmh
+
+on: [push, pull_request]
+
+permissions:
+  contents: read
+
+jobs:
+  test:
+    strategy:
+      fail-fast: false
+      matrix:
+        include:
+          - suite: bench
+            pattern: '\\.bench\\.'
+          - suite: core
+            pattern: '\\.perf\\.[A-Z]'
+          - suite: grails
+            pattern: '\\.perf\\.grails\\.'
+    runs-on: ubuntu-latest
+    env:
+      DEVELOCITY_ACCESS_KEY: ${{ secrets.DEVELOCITY_ACCESS_KEY }}
+    steps:
+      - uses: actions/checkout@v6
+      - uses: actions/setup-java@v5
+        with:
+          distribution: 'zulu'
+          java-version: 21
+          check-latest: true
+      - uses: gradle/actions/setup-gradle@v5
+      - name: JMH (${{ matrix.suite }})
+        run: ./gradlew perf:jmh -PbenchInclude=${{ matrix.pattern }}
+        timeout-minutes: 60
+      - name: Rename JMH result file
+        run: |
+          mv subprojects/performance/build/results/jmh/results.txt \
+             subprojects/performance/build/results/jmh/results-${{ 
matrix.suite }}.txt
+
+      - name: Upload reports-jmh-${{ matrix.suite }}
+        uses: actions/upload-artifact@v6
+        with:

Review Comment:
   This workflow bumps GitHub Actions to `actions/checkout@v6`, 
`actions/setup-java@v5`, `gradle/actions/setup-gradle@v5`, and 
`actions/upload-artifact@v6`, while the rest of the repo predominantly uses 
`@v4` (and `checkout@v5` in wrapper validation). Please align these versions 
with what the repo already uses (or confirm these majors exist and are 
desired), otherwise CI may fail due to unresolved action versions.



##########
subprojects/performance/src/jmh/groovy/org/apache/groovy/perf/OperatorBench.groovy:
##########
@@ -0,0 +1,207 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package org.apache.groovy.perf
+
+import org.openjdk.jmh.annotations.*
+import org.openjdk.jmh.infra.Blackhole
+
+import java.util.concurrent.TimeUnit
+
+/**
+ * Tests the performance of Groovy operator overloading. In Groovy every
+ * operator (+, -, *, /, [], <<, ==, <=>) compiles to a method call
+ * (plus, minus, multiply, div, getAt, leftShift, equals, compareTo)
+ * dispatched through invokedynamic.
+ */
+@Warmup(iterations = 3, time = 2, timeUnit = TimeUnit.SECONDS)
+@Measurement(iterations = 5, time = 2, timeUnit = TimeUnit.SECONDS)
+@Fork(2)
+@BenchmarkMode(Mode.AverageTime)
+@OutputTimeUnit(TimeUnit.MILLISECONDS)
+@State(Scope.Thread)
+class OperatorBench {
+    static final int ITERATIONS = 1_000_000
+
+    /**
+     * Integer addition — dispatches to Integer.plus(Integer).
+     */
+    @Benchmark
+    void integerPlus(Blackhole bh) {
+        int sum = 0
+        for (int i = 0; i < ITERATIONS; i++) {
+            sum = sum + i
+        }
+        bh.consume(sum)
+    }
+
+    /**
+     * Integer multiplication — dispatches to Integer.multiply(Integer).
+     * Uses modulo to keep operands small and avoid overflow to zero.
+     */
+    @Benchmark
+    void integerMultiply(Blackhole bh) {
+        int product = 1
+        for (int i = 1; i < ITERATIONS; i++) {
+            product = (i % 100) * (i % 50)

Review Comment:
   `integerMultiply` overwrites `product` each iteration and only consumes the 
final value at the end. That makes the loop’s intermediate work a candidate for 
dead-code elimination / loop collapsing, which can invalidate the benchmark. 
Consider making `product` depend on its previous value (or consuming within the 
loop) so the work can’t be optimized away.
   ```suggestion
               product *= ((i % 100) * (i % 50) + 1)
   ```



##########
.github/workflows/groovy-jmh-classic.yml:
##########
@@ -0,0 +1,59 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+name: jmh-classic
+
+on: [push, pull_request]
+
+permissions:
+  contents: read
+
+jobs:
+  test:
+    strategy:
+      fail-fast: false
+      matrix:
+        include:
+          - suite: bench
+            pattern: '\\.bench\\.'
+          - suite: core
+            pattern: '\\.perf\\.[A-Z]'
+          - suite: grails
+            pattern: '\\.perf\\.grails\\.'
+    runs-on: ubuntu-latest
+    env:
+      DEVELOCITY_ACCESS_KEY: ${{ secrets.DEVELOCITY_ACCESS_KEY }}
+    steps:
+      - uses: actions/checkout@v6
+      - uses: actions/setup-java@v5
+        with:
+          distribution: 'zulu'
+          java-version: 21
+          check-latest: true
+      - uses: gradle/actions/setup-gradle@v5
+      - name: JMH (${{ matrix.suite }} classic)
+        run: ./gradlew perf:jmh -PbenchInclude=${{ matrix.pattern }} 
-Pindy=false
+        timeout-minutes: 60
+      - name: Rename JMH result file
+        run: |
+          mv subprojects/performance/build/results/jmh/results.txt \
+             subprojects/performance/build/results/jmh/results-${{ 
matrix.suite }}.txt
+
+      - name: Upload reports-jmh-classic-${{ matrix.suite }}
+        uses: actions/upload-artifact@v6
+        with:

Review Comment:
   This workflow uses `actions/checkout@v6`, `actions/setup-java@v5`, 
`gradle/actions/setup-gradle@v5`, and `actions/upload-artifact@v6`, which is 
inconsistent with the rest of the repo’s workflows (mostly `@v4`, with 
`checkout@v5` in wrapper validation). Please standardize these action versions 
(or verify these majors are intentional and available) to avoid CI failures 
from missing action tags.





> Groovy 4 runtime performance on average 2.4x slower than Groovy 3
> -----------------------------------------------------------------
>
>                 Key: GROOVY-10307
>                 URL: https://issues.apache.org/jira/browse/GROOVY-10307
>             Project: Groovy
>          Issue Type: Bug
>          Components: bytecode, performance
>    Affects Versions: 4.0.0-beta-1, 3.0.9
>         Environment: OpenJDK Runtime Environment AdoptOpenJDK-11.0.11+9 
> (build 11.0.11+9)
> OpenJDK 64-Bit Server VM AdoptOpenJDK-11.0.11+9 (build 11.0.11+9, mixed mode)
> WIN10 (tests) / REL 8 (web application)
> IntelliJ 2021.2 
>            Reporter: mgroovy
>            Priority: Major
>         Attachments: groovy_3_0_9_gc.png, groovy_3_0_9_loop2.png, 
> groovy_3_0_9_loop4.png, groovy_3_0_9_mem.png, groovy_4_0_0_b1_loop2.png, 
> groovy_4_0_0_b1_loop4.png, groovy_4_0_0_b1_loop4_gc.png, 
> groovy_4_0_0_b1_loop4_mem.png, 
> groovysql_performance_groovy4_2_xx_yy_zzzz.groovy, loops.groovy, 
> profile3.txt, profile4-loops.txt, profile4.txt, profile4d.txt
>
>
> Groovy 4.0.0-beta-1 runtime performance in our framework is on average 2 to 3 
> times slower compared to using Groovy 3.0.9 (regular i.e. non-INDY)
> * Our complete framework and application code is completely written in 
> Groovy, spread over multiple IntelliJ modules
> ** mixed @CompileDynamic/@TypeChecked and @CompileStatic
> ** No Java classes left in project, i.e. no cross compilation occurs
> * We build using IntelliJ 2021.2 Groovy build process, then run / deploy the 
> compiled class files
> ** We do _not_ use a Groovy based DSL, nor do we execute Groovy scripts 
> during execution
> * Performance degradation when using Groovy 4.0.0-beta-1 instead of Groovy 
> 3.0.9 (non-INDY):
> ** The performance of the largest of our web applications has dropped 3x 
> (startup) / 2x (table refresh) respectively
> *** Stack: Tomcat/Vaadin/Ebean plus framework generated SQL
> ** Our test suite runs about 2.4 times as long as before (120 min when using 
> G4, compared to about 50 min with G3)
> *** JUnit 5 
> *** test suite also contains no scripts / dynamic code execution
> *** Individual test performance varies: A small number of tests runs faster, 
> but the majority is slower, with some extreme cases taking nearly 10x as long 
> to finish
> * Using Groovy 3.0.9 INDY displays nearly identical performance degradation, 
> so it seems that the use of invoke dynamic is somehow at fault



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to