GROOVY-??? CI test to see if a performance tweak to AnnotationNode he…

[paulk-asert]

…lps PerformanceTest speed on CI

This is a small spike to test whether caching isTargetAllowed improves compiler performance.
It shows a very minor improvement at the expense of a minor increase in complexity to AnnotationNode.

Before changes:

Task :performance:performanceTests
Groovy 5.0.5 Average 484.44ms ± 19.31ms
Groovy current Average 549.21ms ± 18.24ms (13.37% slower)

With changes:

Task :performance:performanceTests
Groovy 5.0.5 Average 493.79ms ± 20.33ms
Groovy current Average 551.14ms ± 20.53ms (11.62% slower)

It seems to have only marginal improvement (and CI isn't guaranteed to provide repeatable accuracy) but maybe a few improvements like this could all add up. If it seems worth doing, I'll create a proper Jira ticket.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 90.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 66.5648%. Comparing base (799c8cd) to head (bc31820).
⚠️ Report is 4 commits behind head on master.

Files with missing lines	Patch %	Lines
...n/java/org/codehaus/groovy/ast/AnnotationNode.java	90.0000%	1 Missing ⚠️

Additional details and impacted files

@@                Coverage Diff                 @@
##               master      #2448        +/-   ##
==================================================
+ Coverage     66.5628%   66.5648%   +0.0020%     
- Complexity      30248      30250         +2     
==================================================
  Files            1406       1406                
  Lines          117737     117744         +7     
  Branches        20907      20908         +1     
==================================================
+ Hits            78369      78376         +7     
- Misses          32934      32935         +1     
+ Partials         6434       6433         -1     

Files with missing lines	Coverage Δ
...n/java/org/codehaus/groovy/ast/AnnotationNode.java	72.3881% <90.0000%> (+0.7345%)	⬆️

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[eric-milles]

The value should be cached within the annotation definition class node. Is there an example of an annotation type that is re-computing the targets allowed value over and over? There may be ClassNode duplication for some type.

[paulk-asert]

The performance tests show Groovy 6 slower than Groovy 5. AI analysed all 90ish commits different between the two versions and tried to guess which might be part of the slow down. It saw the body of that method changed to a slower implementation and that the method is called many times. This change was it's number one guess but its overall assessment was that there wasn't any one thing, just lots of little things. I say "guess" because it was analysing code paths not runtime heatmaps. All these things are tradeoffs. Does it make it 1-2% faster but now need slightly more memory?

[eric-milles]

My intent was to get rid of all the old pre-loading of allowed targets so that the annotation type could answer according to its own annotation metadata (or defaults if no target metadata). There was a big gap between binary and source references. It should be closed now. That may cost a bit of time.

Time spent during compilation is different from runtime performance. There are of course some cases of runtime compilation; is that something in common usage?

[paulk-asert]

yeah, I was just looking at compilation performance for that run.