GEP-27 reference implementation (including some parts currently planned for Groovy 7 but could possibly be moved forward)#2683
Draft
paulk-asert wants to merge 1 commit into
Conversation
423a96c to
59a91ae
Compare
…c dispatch Add closure packing on top of the SAM-lambda hoisting: an eligible closure literal's body is hoisted into a synthetic method on the enclosing class and the literal is replaced by a shared PackedClosure adapter, removing the per-closure generated class (and the nested $_closure1$_closure2 name explosion) while the value stays a real groovy.lang.Closure. Capability analysis (PackStrategy, ClosureWriter.chooseStrategy): - a single decision point selects FULL_CLASS (S0) vs PACKED_ADAPTER (S1); - triggered by the @PackedClosures annotation (trust path, dynamic or static), or automatically under @CompileStatic when the type checker PROVES the closure delegate-independent -- every implicitly-received name resolved against the owner, read from STC's IMPLICIT_RECEIVER paths (a path ending in "owner" is safe; "delegate", from @DelegatesTo/with, is not). Behind groovy.target.closure.pack. Typed static dispatch (phase 4): the hoisted body reuses the expressions StaticCompilationVisitor already annotated, so under @CompileStatic it compiles statically -- read-only captures pass as typed leading parameters, inferred types (@ClosureParams, implicit it) arrive via metadata, and the method takes the closure's inferred return type. Packed bodies are statically dispatched with zero invokedynamic, often fewer indy sites than a closure class (which Reference-boxes captures as Object). Written captures ride the compiler's holder machinery: the shared Reference is passed as a parameter (originType/closure-shared/UseExistingReference), so the body is not rewritten, compiles statically, and supports every write form (compound ops beyond +=, postfix value semantics). Soundness for the dynamic trust path: - PackedClosure runtime guard fails fast if a delegate / delegate-consulting resolveStrategy is set on a trust-packed closure; a statically proven closure tolerates it as a no-op (as a @CompileStatic closure class does today); - compile-time escape decline keeps closures that visibly escape (stored to a field/property, returned, appended, in a collection literal) as classes. Mechanism fidelity so a packed closure behaves like a generated closure class: correct owner/static dispatch in static methods; getParameterTypes() carries the real declared types; arguments are adapted as reflective dispatch would (vararg collection, single List/Tuple destructuring, per-arg coercion Closure->SAM and GString->String); call() dispatches straight to doCall with invoker-exception unwrapping; dispatch via a cached reflective Method (metaclass coercion would auto-dereference Reference holder arguments). ClassNode.visitMethods: dedup newly-added methods by identity via an IdentityHashMap set (O(1)) instead of ArrayList.removeAll (O(n^2)). Visiting a method during code generation can add more (constructor-ref synthetic, lambda $deserializeLambda$, a hoisted body that hoists a nested one); the O(1) fixpoint visits them all and fixes a compile-time freeze the O(n^2) scan caused against method-generating visitors (e.g. groovy-contracts). Also use the fluent AstQuery API (GROOVY-12116) for the lambda nested-function check, and skip pre-pack bytecode-shape tests under the flag via @DisabledIfSystemProperty. Tests: ClosurePackCapabilityTest and PackedClosuresTransformTest (proof, declines, auto-pack, zero-indy typed bodies, write forms, destructuring). Flag-on stress net 0 failures across LambdaTest + all *Closure*/*DelegatesTo* static-compile suites + ReproducibleBytecodeBugs; flag-off byte-identical (778 green); groovy-contracts compiles and tests clean.
59a91ae to
d351610
Compare
✅ All tests passed ✅🏷️ Commit: d351610 Learn more about TestLens at testlens.app. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
GEP-27: Compact Closure & Lambda Compilation — closure packing, capability analysis & typed static dispatch (draft/RFC)
Status
Draft for review of the GEP-27 direction. Two clean commits: the Groovy-6 lambda slice (
GROOVY-12143, raised separately) as the base, and the larger closure-side work (originally scoped for Groovy 7) on top. Opt-in behind flags, off by default, byte-for-byte today's behaviour when disabled. Posted to validate the approach end-to-end and gather feedback.What this delivers
Beyond the lambda leg (which eliminates the generated class for SAM lambdas — see
GROOVY-12143), this branch implements closure packing: an eligible closure literal's body is hoisted into a synthetic method on the enclosing class and the literal is replaced by a single sharedPackedClosureadapter — removing the per-closure generated class (and the deeply-nested$_closure1$_closure2$_closure3name explosion) while the runtime value stays a realgroovy.lang.Closure(socurry/memoize/trampoline/iteration keep working).1. Capability analysis (
PackStrategy,chooseStrategy). A single decision point selectsFULL_CLASS(S0) vsPACKED_ADAPTER(S1) — shared vocabulary for the closure and lambda writers.2. Two ways to trigger packing:
@PackedClosuresannotation (class/method) — the opt-in trust path (dynamic or static).@CompileStatic—chooseStrategyproves delegate-independence using the type checker's ownIMPLICIT_RECEIVERresolution paths (a path ending inowneris owner-resolved and safe;delegate, from@DelegatesTo/with, is not), so packing is sound with no annotation and no trust. Behindgroovy.target.closure.pack.3. Typed static dispatch (GEP-27 phase 4). The hoisted body reuses the expressions
StaticCompilationVisitoralready annotated, so under@CompileStaticit compiles statically — read-only captures pass as typed leading parameters, inferred types (@ClosureParams, implicitit) arrive via metadata, and the method takes the closure's inferred return type. Packed bodies are statically dispatched, zeroinvokedynamic— often fewer indy sites than today's closure class, whichReference-boxes captures asObject.4. Written captures ride the compiler's existing holder machinery (the shared
Referencepassed as a parameter), so the body is never rewritten, compiles statically, and supports every write form (compound ops beyond+=, postfix value semantics) — replacing ~70 lines of AST-rewrite with fewer, more capable ones.5. Soundness for the dynamic trust path:
PackedClosurefails fast (source-located message) if a delegate / delegate-consultingresolveStrategyis set on a trust-packed closure; a statically proven closure tolerates it as a no-op (as a@CompileStaticclosure class does today).static constraints = { … }shape) stay classes.6. Mechanism fidelity so a packed closure behaves like a generated closure class: correct owner/static dispatch in
staticmethods;getParameterTypes()carries the real declared types; arguments adapted as reflective dispatch would (vararg collection, singleList/Tupledestructuring,Closure→SAM /GString→Stringcoercion);call()dispatches straight todoCallwith invoker-exception unwrapping.7.
ClassNode.visitMethods— O(1) fixpoint. Visiting a method during code generation can add more (constructor-ref synthetic, lambda$deserializeLambda$, a hoisted body that hoists a nested one); dedup newly-added methods by identity via anIdentityHashMapset (O(1)) instead ofArrayList.removeAll(O(n²)). This fixes a compile-time freeze the O(n²) scan caused against method-generating visitors (e.g. groovy-contracts).Flags
groovy.target.lambda.hoistGROOVY-12143)groovy.target.closure.pack@CompileStaticclosure packing@PackedClosuresThe build forwards
groovy.target.closure.packto both the forked Groovy compiler (compile-time packing of sources) and the test JVM (runtimeassertScript). Flags off ⇒ byte-for-byte today's behaviour.Testing
ClosurePackCapabilityTest,PackedClosuresTransformTest(delegate-independence proof,@DelegatesTodeclines, auto-pack, zero-indy typed bodies, write forms,List/Tupledestructuring), plusLambdaHoistTest.LambdaTest+ every*Closure*/*DelegatesTo*static-compile suite +ReproducibleBytecodeBugs), diffed by failing-test set against the dynamic-body baseline (169 → 0 over the increments).groovy-core(678-test stress green),groovy-contracts(full suite green, incl. the case that previously froze), andgroovy-json/groovy-xml/groovy-sql/groovy-templates/groovy-datetime(compileTestGroovyclean, zeroVerifyErrors).Known limitations / follow-ups
Method(prototype); aninvokedynamiccall site is the known performance upgrade.clean testwith the flag on across all ~60 modules, and interactive IDE/tooling verification (debugger step-into, breakpoints, variable inspection; coverage/decompiler tools), remain before any default flip.Relationship to the lambda PR
Includes
GROOVY-12143as its base (lambda S2). If that lands first, this branch rebases to just the closure-side commit.