[
https://issues.apache.org/jira/browse/GROOVY-12642?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18136262#comment-18136262
]
ASF GitHub Bot commented on GROOVY-12642:
-----------------------------------------
paulk-asert opened a new pull request, #3163:
URL: https://github.com/apache/groovy/pull/3163
…k while looping
A spread-dot expression under static compilation is generated as an inline
loop that adds each value to a new ArrayList. That list was kept on the operand
stack for the whole loop, as the value of the expression, so the loop's back
edge carried a non-empty stack and HotSpot could not compile the running loop
on-stack: a method run once over a large collection stayed in the interpreter.
The list now lives only in its temporary slot during the loop and is loaded
from it afterwards, so the stack is empty at the back edge whenever nothing
else is pending. A spread evaluated after other operands, such as a second
argument, still loops with those operands on the stack.
> Statically compiled spread-dot loops keep the result list on the operand
> stack, which blocks HotSpot on-stack replacement
> -------------------------------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12642
> URL: https://issues.apache.org/jira/browse/GROOVY-12642
> Project: Groovy
> Issue Type: Improvement
> Components: Static compilation
> Affects Versions: 6.0.0
> Reporter: Paul King
> Priority: Major
>
> Description:
> Under @CompileStatic, a spread-dot expression such as `ps*.x` or `ps*.twice()`
> is compiled inline as a for-in loop that adds each value to a new ArrayList.
> The generated code keeps a copy of that ArrayList on the JVM operand stack for
> the whole loop (it is the value of the expression), and the loop's back edge
> therefore carries a non-empty stack:
> {code}
> 8: new ArrayList / dup / invokespecial <init>
> 15: dup
> 16: astore_1 // list stored in a slot, a second copy left on the
> stack
> ...
> 42: iterator.hasNext() ... aload_1, element.length(), ArrayList.add, pop
> 89: goto 42 // back edge with the list still on the stack
> {code}
> HotSpot cannot compile a running loop on-stack (OSR) when the operand stack at
> the loop is not empty. With -XX:+PrintCompilation on JDK 17 both tiers refuse:
> {code}
> Bench::spread @ 42 (97 bytes) COMPILE SKIPPED: stack not empty at OSR entry
> point (retry at different tier)
> Bench::spread @ 42 (97 bytes) COMPILE SKIPPED: OSR starts with non-empty
> stack (retry at different tier)
> {code}
> A method that is called often is unaffected, since it is compiled as a whole
> on a later call, but a method that runs once over a large collection (a script
> body, main(), batch or pipeline setup, a benchmark) runs the whole traversal
> in the interpreter. Over 3 million elements, a one-shot `def r = ps*.x` took
> 464 ms against 30 ms for the equivalent hand-written loop (6.0.0-beta-2,
> JDK 17).
> This was first reported in a blog post on Groovy with Flink on TornadoVM,
> where `batch*.f1` over a million elements took around 400 ms and a plain loop
> was around ten times faster.
> Dynamic code, including @TypeChecked, is not affected: it calls the
> ScriptBytecodeAdapter *SpreadSafe helpers, whose loops are ordinary Java
> loops.
> Proposed fix:
> In StaticInvocationWriter.makeCall, keep the result list only in its temporary
> slot while looping and load it from the slot after the loop. The operand stack
> is then empty at the back edge whenever the spread is evaluated with nothing
> else pending on the stack, which covers the usual forms:
> `def r = xs*.f`, `r = xs*.f`, `return xs*.f`, and a spread used as a receiver
> (`xs*.f.sum()`). Evaluation order, null handling and the result are unchanged.
> Not covered: a spread evaluated while other operands are already on the stack,
> for example an argument after another argument (`foo(a, xs*.f)`) or the right
> operand of a binary expression (`n + xs*.f.size()`). Those would need the
> pending stack spilled to locals or the loop moved out of line, and are left
> for a follow-up if worthwhile.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
