Paul King created GROOVY-12642:
----------------------------------
Summary: 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
Reporter: Paul King
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)
