[
https://issues.apache.org/jira/browse/GROOVY-12726?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18140258#comment-18140258
]
ASF GitHub Bot commented on GROOVY-12726:
-----------------------------------------
github-actions[bot] commented on PR #3262:
URL: https://github.com/apache/groovy/pull/3262#issuecomment-6706911863
### JMH summary — indy (commit `d1a19d0`)
Speedup vs trailing 90-day baseline on gh-pages. Higher = faster.
`1.00` = in line with history. Per-benchmark ratio, geomean within group.
Time-per-op units inverted so direction is consistent. The *calibrated*
column divides out this runner's speed vs the baseline hardware, as
measured by Groovy-independent pure-Java ruler benchmarks.
| Group | Speedup | Calibrated | n |
|--------|---------|------------|---|
| bench | 1.659 × | 0.954 × | 124 |
| core | 3.201 × | 2.465 × | 108 |
| grails | 1.009 × | 1.162 × | 80 |
> ⚠️ **1 benchmark at least 1.5× slower than the 90-day baseline:**
> - `org.apache.groovy.bench.AryBench.groovyCS ( {"n":"10"} )` — 1.69×
slower (calibrated)
> ⚠️ Runner speed differs ≥15% from the historical baseline hardware for:
bench, core-ag, core-hz, grails-ez. Raw speedups are not meaningful for those
parts — use the calibrated column.
<sub>Runner calibration (this run vs baseline hardware): bench 1.71× (27
rulers) · core-ag 1.24× (3 rulers) · core-hz 1.41× (3 rulers) · grails-ad 0.87×
(3 rulers) · grails-ez 0.87× (3 rulers)</sub>
<sub>Baseline: <code>dev/bench/jmh/<part>/indy/data.js</code> on
gh-pages, trailing 90 days. <a
href="https://apache.github.io/groovy/dev/bench/jmh/summary.html";>Daily
dashboard</a> · <a
href="https://apache.github.io/groovy/dev/bench/jmh/";>Per-suite raw
data</a></sub>
<!
> Fix joint-compilation stubber placeholder cleanup and field-level
> @Bindable/@Vetoable stubbing
> ----------------------------------------------------------------------------------------------
>
> Key: GROOVY-12726
> URL: https://issues.apache.org/jira/browse/GROOVY-12726
> Project: Groovy
> Issue Type: Bug
> Reporter: Daniel Sun
> Assignee: Daniel Sun
> Priority: Major
>
> h2. Overview
> During joint compilation, stubber AST transformations
> ({{ToStringASTStubber}}, {{EqualsAndHashCodeASTStubber}},
> {{IndexedPropertyASTStubber}}, {{BindableASTStubber}}, and
> {{VetoableASTStubber}}) run in the {{CONVERSION}} phase to insert placeholder
> {{MethodNode}} stubs (marked via {{StubberSupport}}) so that the Java stub
> generator can emit method signatures before the full AST transformations run
> in later phases. Several edge cases in the handoff between
> {{CONVERSION}}-phase stubbers and their corresponding full AST
> transformations can leave dummy stub methods in final bytecode or omit
> required methods from Java stubs.
> h2. Problem Description
> h3. 1. {{@ToString}} and {{@EqualsAndHashCode}} leak dummy stub methods when
> {{_toString}}, {{_hashCode}}, or {{_equals}} is declared
> In {{ToStringASTTransformation.createToString}} and
> {{EqualsAndHashCodeASTTransformation}} ({{createHashCode}} and
> {{createEquals}}), the presence of a {{CONVERSION}}-phase stub sets
> {{hasExistingToString}}, {{hasExistingHashCode}}, or {{hasExistingEquals}} to
> {{true}}. However, the transformations checked {{hasDeclaredMethod(cNode,
> UNDER_TO_STRING, 0)}} (and {{UNDER_HASH_CODE}} / {{UNDER_EQUALS}}) and
> returned early *before* checking {{StubberSupport.isStub(...)}}:
> {code:java}
> if (hasExistingToString) {
> if (hasDeclaredMethod(cNode, UNDER_TO_STRING, 0)) return;
> MethodNode toString = cNode.getDeclaredMethod(TO_STRING,
> Parameter.EMPTY_ARRAY);
> if (StubberSupport.isStub(toString)) {
> stubberPlaceholder = toString;
> hasExistingToString = false;
> }
> ...
> }
> {code}
> As a result, if a class annotated with {{@ToString}} or
> {{@EqualsAndHashCode}} also declared a helper method ({{_toString()}},
> {{_hashCode()}}, or {{_equals(Object)}}), the transformation returned early
> during {{CANONICALIZATION}} without replacing the stubber placeholder,
> leaving the dummy stub implementation ({{return ""}}, {{return 0}}, or
> {{return false}}) in the compiled class.
> h3. 2. {{@IndexedProperty}} leaks setter stubs on immutable classes
> {{IndexedPropertyASTStubber}} runs in {{CONVERSION}}, before meta-annotations
> (such as a custom {{@AnnotationCollector}} expanding to {{@ImmutableBase}})
> are expanded in {{SEMANTIC_ANALYSIS}}, and therefore emits both {{getX(int)}}
> and {{setX(int, V)}} placeholders. Later, in {{CANONICALIZATION}},
> {{IndexedPropertyASTTransformation}} detects {{IMMUTABLE_BREADCRUMB}} on the
> field and skips {{addArraySetter}} / {{addListSetter}}, but did not remove
> the {{CONVERSION}}-phase setter placeholder, leaving a no-op {{setX(int, V)}}
> method in the bytecode of the immutable class. In addition,
> {{IndexedPropertyASTStubber.isClassImmutable}} did not check for
> {{@RecordType}} or {{@RecordBase}}.
> h3. 3. {{BindableASTTransformation}} and {{VetoableASTTransformation}} do not
> strip stub placeholders in {{needsPropertyChangeSupport}} /
> {{needsVetoableChangeSupport}}
> {{BindableASTTransformation.needsPropertyChangeSupport}} and
> {{VetoableASTTransformation.needsVetoableChangeSupport}} scan
> {{declaringClass.getMethods()}} for existing listener registration and firing
> methods without first removing {{StubberSupport.isStub(m)}} placeholders,
> which can cause them to treat stubber placeholders as user-supplied
> implementations.
> h3. 4. {{BindableASTStubber}} and {{VetoableASTStubber}} ignore field-level
> {{@Bindable}} and {{@Vetoable}} annotations
> {{@Bindable}} and {{@Vetoable}} can be applied to individual fields as well
> as classes, and both forms generate class-level listener methods
> ({{addPropertyChangeListener}}, {{addVetoableChangeListener}}, etc.).
> However, {{BindableASTStubber}} and {{VetoableASTStubber}} returned early
> whenever {{!(parent instanceof ClassNode)}}, so Java sources in a joint
> compilation unit could not call listener methods on Groovy classes that only
> used field-level {{@Bindable}} or {{@Vetoable}}.
> h2. Solution
> * Check {{StubberSupport.isStub(...)}} before checking for {{_toString}},
> {{_hashCode}}, and {{_equals}} in {{ToStringASTTransformation}} and
> {{EqualsAndHashCodeASTTransformation}} so stubber placeholders are always
> replaced with the generated implementation.
> * Remove any stubber-generated indexed setter placeholder in
> {{IndexedPropertyASTTransformation}} when {{immutable}} is {{true}}, and
> include {{@RecordType}} and {{@RecordBase}} in
> {{IndexedPropertyASTStubber.isClassImmutable}}.
> * Remove stubber placeholders at the start of
> {{BindableASTTransformation.needsPropertyChangeSupport}} and
> {{VetoableASTTransformation.needsVetoableChangeSupport}}.
> * Update {{BindableASTStubber}} and {{VetoableASTStubber}} to also emit
> listener method stubs on the declaring class when {{@Bindable}} or
> {{@Vetoable}} annotates a non-static, non-final property field.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
