[
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)