[
https://issues.apache.org/jira/browse/GROOVY-12750?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Paul King updated GROOVY-12750:
-------------------------------
Description:
Follow-up fixes to closure packing (GROOVY-12151), which shipped in 6.0.0.
h3. A direct call unwrapped an InvokerInvocationException thrown by the closure
body
The {{call(Object)}} and {{call(Object...)}} paths of a packed closure reach
the hoisted body through its dispatch table, with no reflective step in
between, so there is no invoker wrapping to undo. They nevertheless caught
{{InvokerInvocationException}} and rethrew its cause, so an
{{InvokerInvocationException}} thrown by the body itself was unwrapped, where a
closure compiled to its own class lets it through:
{code:groovy}
// with closure packing enabled
Closure one = { Object x -> throw new InvokerInvocationException(new
IOException('one')) }
one.call((Object) 1) // closure class: InvokerInvocationException; packed
closure: IOException
{code}
The catch is removed from both paths. {{mopCall}}, which goes through the
metaclass, keeps it.
h3. Fixed-arity calls allocated the closure on every creation
A dynamic call site invokes a packed closure through its fixed-arity
{{doCall}}, which wrapped the arguments in an array only for {{dispatchAll}} to
check the length against the class's constant arity and unwrap them again.
Escape analysis removes that array, but the {{doCall}} -> {{dispatchAll}} chain
was too large to inline at the call site, so the closure object itself escaped
and was allocated each time it was created. The one-parameter and
implicit-parameter forms now go straight to {{dispatchOne}}, and the
two-parameter form to a new {{dispatchTwo}}, the branches {{dispatchAll}} takes
for these shapes anyway.
A new JMH benchmark, {{ClosureDoCallBench}} (dynamic {{c(x)}} call sites,
{{-prof gc}}), shows the monomorphic one- and two-parameter cases going from
0.6-0.7x to about 1.0x of a closure compiled to its own class, and from 128 to
32 B/op. The implicit-parameter case improves in most forks, and the
megamorphic case is unchanged.
h3. The packing runtime link was missing from the @GroovyABI registry
A class compiled with closure packing links against parts of the runtime that
the {{@GroovyABI}} registry did not record: the {{MethodHandle}}-taking
{{IndyInterface.packedDispatchers}} bootstrap, the
{{GeneratedDispatcher$Bundle}} constructor its dispatcher factory calls, and
the {{dispatch}}/{{dispatch1}}/{{dispatch2}} methods its {{LambdaMetafactory}}
sites implement. They are now annotated {{@GroovyABI(since = "6.0.0")}} and
registered, so that {{checkGroovyABISync}} catches a change that would break
compiled code. Comments claiming that {{Bundle}} and {{GeneratedDispatcher}}
never appear in generated class files are corrected.
h3. Backport
All three changes apply to 6.0.x.
was:
Follow-up fixes to closure packing (GROOVY-12151), which shipped in 6.0.0.
h3. A direct call unwrapped an InvokerInvocationException thrown by the closure
body
The {{call(Object)}} and {{call(Object...)}} paths of a packed closure reach
the hoisted body through its dispatch table, with no reflective step in
between, so there is no invoker wrapping to undo. They nevertheless caught
{{InvokerInvocationException}} and rethrew its cause, so an
{{InvokerInvocationException}} thrown by the body itself was unwrapped, where a
closure compiled to its own class lets it through:
{code:groovy}
// with closure packing enabled
Closure one = { Object x -> throw new InvokerInvocationException(new
IOException('one')) }
one.call((Object) 1) // closure class: InvokerInvocationException; packed
closure: IOException
{code}
The catch is removed from both paths. {{mopCall}}, which goes through the
metaclass, keeps it.
h3. Fixed-arity calls allocated the closure on every creation
A dynamic call site invokes a packed closure through its fixed-arity
{{doCall}}, which wrapped the arguments in an array only for {{dispatchAll}} to
check the length against the class's constant arity and unwrap them again.
Escape analysis removes that array, but the {{doCall}} -> {{dispatchAll}} chain
was too large to inline at the call site, so the closure object itself escaped
and was allocated each time it was created. The one-parameter and
implicit-parameter forms now go straight to {{dispatchOne}}, and the
two-parameter form to a new {{dispatchTwo}}, the branches {{dispatchAll}} takes
for these shapes anyway.
A new JMH benchmark, {{ClosureDoCallBench}} (dynamic {{c(x)}} call sites,
{{-prof gc}}), shows the monomorphic one- and two-parameter cases going from
0.6-0.7x to about 1.0x of a closure compiled to its own class, and from 128 to
32 B/op. The implicit-parameter case improves in most forks, and the
megamorphic case is unchanged.
h3. The packing runtime link was missing from the @GroovyABI registry
A class compiled with closure packing links against parts of the runtime that
the {{@GroovyABI}} registry did not record: the {{MethodHandle}}-taking
{{IndyInterface.packedDispatchers}} bootstrap, the
{{GeneratedDispatcher$Bundle}} constructor its dispatcher factory calls, and
the {{dispatch}}/{{dispatch1}}/{{dispatch2}} methods its {{LambdaMetafactory}}
sites implement. They are now annotated {{@GroovyABI(since = "6.0.0")}} and
registered, so that {{checkGroovyABISync}} catches a change that would break
compiled code. Comments claiming that {{Bundle}} and {{GeneratedDispatcher}}
never appear in generated class files are corrected.
h3. Backport
The first two fixes apply to 6.0.x. The registry change concerns the
compatibility tracking on master.
> Closure packing: dispatch fixed-arity calls without an argument array, and
> let a direct call propagate an InvokerInvocationException the body throws
> ----------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12750
> URL: https://issues.apache.org/jira/browse/GROOVY-12750
> Project: Groovy
> Issue Type: Bug
> Reporter: Paul King
> Priority: Major
>
> Follow-up fixes to closure packing (GROOVY-12151), which shipped in 6.0.0.
> h3. A direct call unwrapped an InvokerInvocationException thrown by the
> closure body
> The {{call(Object)}} and {{call(Object...)}} paths of a packed closure reach
> the hoisted body through its dispatch table, with no reflective step in
> between, so there is no invoker wrapping to undo. They nevertheless caught
> {{InvokerInvocationException}} and rethrew its cause, so an
> {{InvokerInvocationException}} thrown by the body itself was unwrapped, where
> a closure compiled to its own class lets it through:
> {code:groovy}
> // with closure packing enabled
> Closure one = { Object x -> throw new InvokerInvocationException(new
> IOException('one')) }
> one.call((Object) 1) // closure class: InvokerInvocationException; packed
> closure: IOException
> {code}
> The catch is removed from both paths. {{mopCall}}, which goes through the
> metaclass, keeps it.
> h3. Fixed-arity calls allocated the closure on every creation
> A dynamic call site invokes a packed closure through its fixed-arity
> {{doCall}}, which wrapped the arguments in an array only for {{dispatchAll}}
> to check the length against the class's constant arity and unwrap them again.
> Escape analysis removes that array, but the {{doCall}} -> {{dispatchAll}}
> chain was too large to inline at the call site, so the closure object itself
> escaped and was allocated each time it was created. The one-parameter and
> implicit-parameter forms now go straight to {{dispatchOne}}, and the
> two-parameter form to a new {{dispatchTwo}}, the branches {{dispatchAll}}
> takes for these shapes anyway.
> A new JMH benchmark, {{ClosureDoCallBench}} (dynamic {{c(x)}} call sites,
> {{-prof gc}}), shows the monomorphic one- and two-parameter cases going from
> 0.6-0.7x to about 1.0x of a closure compiled to its own class, and from 128
> to 32 B/op. The implicit-parameter case improves in most forks, and the
> megamorphic case is unchanged.
> h3. The packing runtime link was missing from the @GroovyABI registry
> A class compiled with closure packing links against parts of the runtime that
> the {{@GroovyABI}} registry did not record: the {{MethodHandle}}-taking
> {{IndyInterface.packedDispatchers}} bootstrap, the
> {{GeneratedDispatcher$Bundle}} constructor its dispatcher factory calls, and
> the {{dispatch}}/{{dispatch1}}/{{dispatch2}} methods its
> {{LambdaMetafactory}} sites implement. They are now annotated
> {{@GroovyABI(since = "6.0.0")}} and registered, so that
> {{checkGroovyABISync}} catches a change that would break compiled code.
> Comments claiming that {{Bundle}} and {{GeneratedDispatcher}} never appear in
> generated class files are corrected.
> h3. Backport
> All three changes apply to 6.0.x.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
