[
https://issues.apache.org/jira/browse/GROOVY-10307?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18076367#comment-18076367
]
ASF GitHub Bot commented on GROOVY-10307:
-----------------------------------------
Copilot commented on code in PR #2494:
URL: https://github.com/apache/groovy/pull/2494#discussion_r3143914078
##########
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)
+ }
+ bh.consume(product)
Review Comment:
In `integerMultiply`, the loop assigns `product = (i % 100) * (i % 50)` each
iteration rather than accumulating, so the preceding comment about avoiding
overflow in an accumulated product doesn’t match the code. Either adjust the
comment, or change the benchmark to actually accumulate if that’s the intent.
##########
build-logic/src/main/groovy/org.apache.groovy-performance.gradle:
##########
@@ -65,11 +68,16 @@ tasks.named('jmhClasses') {
dependsOn tasks.named('clean')
}
+var indy = (project.findProperty('indy') ?: 'true').toBoolean()
Review Comment:
`var indy = ...` is not valid in Groovy/Gradle scripts ("var" is treated as
a type name / reserved keyword) and will fail to compile this precompiled
script plugin. Use `def indy = ...` (or `boolean indy = ...`) instead.
```suggestion
boolean indy = (project.findProperty('indy') ?: 'true').toBoolean()
```
##########
.github/workflows/groovy-jmh.yml:
##########
@@ -0,0 +1,62 @@
+# 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:
+ if: >-
+ contains(github.event.head_commit.message || '', 'GROOVY-') ||
+ contains(github.event.pull_request.title || '', 'GROOVY-')
+ 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@0723195856401067f7a2779048b490ace7a47d7c # v5.0.2
+ - 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@v7
Review Comment:
This workflow uses `actions/upload-artifact@v7`, but the rest of the repo’s
workflows pin `actions/upload-artifact@v4` (e.g.
`.github/workflows/groovy-build-test.yml`). Please align the major version here
as well to avoid relying on an inconsistent/possibly unavailable action version.
```suggestion
uses: actions/upload-artifact@v4
```
##########
.github/workflows/groovy-jmh-classic.yml:
##########
@@ -0,0 +1,62 @@
+# 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:
+ if: >-
+ contains(github.event.head_commit.message || '', 'GROOVY-') ||
+ contains(github.event.pull_request.title || '', 'GROOVY-')
+ 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@0723195856401067f7a2779048b490ace7a47d7c # v5.0.2
+ - 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@v7
Review Comment:
This workflow uses `actions/upload-artifact@v7`, but the rest of the repo’s
workflows pin `actions/upload-artifact@v4` (e.g.
`.github/workflows/groovy-build-test.yml`). Please align the major version here
as well to avoid relying on an inconsistent/possibly unavailable action version.
```suggestion
uses: actions/upload-artifact@v4
```
> 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)
