Repository navigation
Painless: cancellation_aware augmentation infrastructure - #150092
Conversation
Introduce the SPI annotation @cancellation_aware and the lookup/codegen plumbing that backs it, plus the first augmentation conversion (Iterable.each). When a whitelist line carries the annotation AND the current script context's base class supports cancellation, PainlessLookupBuilder resolves the call to an augmentation overload taking a synthetic leading PainlessScript parameter; the compiler emits aload 0 ahead of the user args at every static call site, and the augmentation body fetches the cancel Runnable via _getCancellationCheck() and polls it every 1000 iterations from inside its driver loop. In non-cancellation-aware contexts the annotation is silently dropped during lookup — the call resolves to whatever the line would have resolved to without it (existing augmentation overload or direct JDK method). Non-cancellation-aware contexts pay zero overhead. The PainlessLookup also collects the user-visible names of cancellation- aware methods into a set, exposed via hasCancellationAwareMethodName, which the upcoming def call-site emission will consult to gate the script-this push. This commit ships: - CancellationAwareAnnotation + parser, registered in BASE_ANNOTATION_PARSERS - ScriptClassInfo.supportsCancellation(Class) static helper - PainlessLookupBuilder.buildFromWhitelists now takes the script base class and routes cancellation-aware lookups through a PainlessScript-inserted parameter list - DefaultIRTreeToASMBytesPhase.visitInvokeCall checks the annotation and pushes script-this - Augmentation.each gets the cancellation-aware overload alongside the existing one - java.lang.txt marks Iterable.each as @cancellation_aware - AugmentationCancellationTests covers the fast-path (no runnable set) and slow-path (poll fires from inside the augmentation driver loop) Static call sites only. Def call-site support (recipe prefix + MethodHandles.dropArguments fallback for non-cancellation-aware overloads) is the next commit.
Extend Phase 1's static-call-site handling to def-dispatched call sites so def-typed receivers (def l = ...; l.each(...)) also route through the cancellation-aware augmentation overload. Compile-time: - IRDecorations gains IRCMaybeNeedsScriptThis, attached during IR construction to a def call when the method name is in the lookup's cancellationAwareMethodNames set. The set is populated only for cancellation-aware contexts, so the condition never fires in others. - visitInvokeCallDef checks the condition AND that the enclosing function caches #cancelRunnable. When both are true: emits aload 0 (script this) ahead of user args, prefixes the recipe with 'S', and adds ScriptThis.class to the descriptor's typeParameters. Runtime: - DefBootstrap.bootstrap peels the 'S' prefix before counting lambdas so the args.length validation isn't off by one. - Def.lookupMethod peels the prefix, decrements numArguments, and threads the offset through the lambda-binding loop's descriptor indexing. After resolving the target method, if it doesn't carry CancellationAwareAnnotation the handle is wrapped with MethodHandles.dropArguments to discard the script-this slot — this handles the false-positive case where a user class happens to share a name with a cancellation-aware augmentation. Per-call cost in cancellation-aware contexts: one aload 0 per def call whose method name is in the cancellation-aware set (small set; gated out for unrelated names like toString, get, etc). Non-cancellation- aware contexts: zero — the set is empty so the condition is never set. Tests cover both the happy path (def each() through DefBootstrap fires the runnable from inside the augmentation loop) and the gate (def toString on an unrelated method name does not push script-this).
Restructure how @cancellation_aware augmentations pass the script receiver so a future method-ref support layer can reuse FunctionRef.withSyntheticScriptCapture (which prepends at position 0 of factoryMethodType) directly, without position arithmetic. Augmentation methods now use the signature method(PainlessScript script, receiver, ...userArgs) instead of the original Phase 1 layout (receiver, PainlessScript, ...). Lookup: - PainlessLookupBuilder.addPainlessMethod inserts PainlessScript at the beginning of the Java parameter list (before the receiver slot), not after, when @cancellation_aware fires. Static call site: - visitCall, for @cancellation_aware augmentations, folds the prefix expression into argumentNodes[0] of the InvokeCallNode instead of wrapping the call in a BinaryImplNode. visitInvokeCall pushes loadThis() first, then the regular arg loop visits the prefix and user args in order. The resulting stack at invokestatic is [scriptThis, receiver, ...userArgs] — direct, no swap. Def call site: - Bytecode shape is unchanged from Phase 2 (descriptor still receiver- first so PIC class dispatch keys off the actual receiver). Def .lookupMethod now reconciles the script-first handle to the receiver- first descriptor with a new swapFirstTwoArguments helper that calls MethodHandles.permuteArguments on the resolved method handle. When the resolved method doesn't carry @cancellation_aware (e.g. a user class shadowing the augmentation name) we still drop the scriptThis slot via dropArguments. Augmentation.each: signature flipped to (PainlessScript, Iterable, Consumer). Tests: existing AugmentationCancellationTests (static + def variants) all green. Method-ref support for @cancellation_aware augmentations is the next phase — the script-first layout means it can reuse FunctionRef.withSyntheticScriptCapture without inventing a position- aware variant.
A method ref like `l::each` to a @cancellation_aware augmentation now threads the script receiver as a synthetic leading FunctionRef capture via withSyntheticScriptCapture, matching the script-first augmentation signature so LambdaBootstrap finds the cancellation-aware overload. DefaultSemanticAnalysisPhase.visitFunctionRef detects when a bound (`obj::method`) or unbound (`Type::method`) ref resolves to such an augmentation and applies the synthetic capture plus InstanceCapturingFunctionRef so the IR phase emits loadThis() at the construction site. FunctionRef.create strips the leading PainlessScript from delegateMethodType before the factory/delegate split so the rest of the lambda linkage sees the user-visible shape.
Cover the paths that ensure the synthetic PainlessScript parameter is only added where needed: non-cancellation contexts resolve the plain each() overload (behavioral + bytecode), the two lookup-builder guards reject misuse, and the def-dispatch fallback drops the script slot when a shadowing receiver method is not cancellation-aware.
|
Pinging @elastic/es-core-infra (Team:Core/Infra) |
|
Hi @jdconrad, I've created a changelog YAML for you. |
🔍 Preview links for changed docs⏳ Building and deploying preview... View progress This comment will be updated with preview links when the build is complete. |
ℹ️ Important: Docs version tagging👋 Thanks for updating the docs! Just a friendly reminder that our docs are now cumulative. This means all 9.x versions are documented on the same page and published off of the main branch, instead of creating separate pages for each minor version. We use applies_to tags to mark version-specific features and changes. Expand for a quick overviewWhen to use applies_to tags:✅ At the page level to indicate which products/deployments the content applies to (mandatory) What NOT to do:❌ Don't remove or replace information that applies to an older version 🤔 Need help?
|
prdoyle
left a comment
There was a problem hiding this comment.
I didn't see anything that looks wrong, at least to the extent that I understand what I'm seeing. 😅
| if (cancel == null) { | ||
| receiver.forEach(consumer); | ||
| return receiver; | ||
| } |
There was a problem hiding this comment.
Not a bad idea: call the one the JIT knows about if you can.
There was a problem hiding this comment.
I do wonder if this would be better suited as a change to the allow lists for contexts that have cancellation vs don't, but definitely adds complexity. Stick with this for now.
| public boolean hasCancellationAwareMethodName(String methodName) { | ||
| return cancellationAwareMethodNames.contains(methodName); |
There was a problem hiding this comment.
(I note this is just looking at the method name and is ignoring arity.)
There was a problem hiding this comment.
Added arity for a stronger awareness, but still doesn't technically remove all ambiguity.
| // MethodHandles.dropArguments (non-cancellation-aware resolution, e.g. a user class | ||
| // shadowing the method name). | ||
| boolean pushScriptThis = irInvokeCallDefNode.hasCondition(IRCMaybeNeedsScriptThis.class) | ||
| && writeScope.getInternalVariable("cancelRunnable") != null; |
There was a problem hiding this comment.
Unless I'm mistaken, this is required to match exactly the conditions under which the script is expecting the extra argument, which I think happens at line 645 of PainlessLookupBuilder. I don't know enough about TRCMaybeNeedsScriptThis or the cancelRunnable internal variable to tell whether this condition is exactly right.
There was a problem hiding this comment.
Kind of went down the rabbit hole with this now. Trying to make this simpler. I think it was correct, but it's awfully complex.
| methodWriter.loadThis(); | ||
| } |
There was a problem hiding this comment.
Again this seems different from the conditions used to compute pushScriptThis but I can't tell if it's wrong.
There was a problem hiding this comment.
Same comment as above.
| Class<?> first = type.parameterType(0); | ||
| Class<?> second = type.parameterType(1); | ||
| // New type has the first two parameters swapped; the rest are unchanged. | ||
| MethodType swapped = type.changeParameterType(0, second).changeParameterType(1, first); | ||
| int[] reorder = new int[type.parameterCount()]; | ||
| reorder[0] = 1; | ||
| reorder[1] = 0; | ||
| for (int i = 2; i < reorder.length; i++) { | ||
| reorder[i] = i; | ||
| } | ||
| return MethodHandles.permuteArguments(handle, swapped, reorder); |
There was a problem hiding this comment.
MethodHandles are fun, aren't they? 😬
There was a problem hiding this comment.
I promise the ordering is correct :)
There was a problem hiding this comment.
Everything in MethodHandles looks backward. It's like talking to Yoda.
Want a handle with fewer arguments? Call insertArguments.
There was a problem hiding this comment.
I try to use permuteArguments whenever it's applicable. I find it more intuitive than all the others: you think about how you'd call one method from the other:
void caller(a0, a1, a2) {
callee(a1, a0, a2);
}
That arg list becomes your array: [1,0,2]. You can even repeat or drop the arguments (so dropArguments isn't really needed):
void caller(a0, a1, a2) {
callee(a1, a0, a2, a1, a1, a1, a2);
}
| } | ||
|
|
||
| // The call-site descriptor has (receiver, scriptThis, ...userArgs) — receiver-first | ||
| // so the PIC's class dispatch (args[0]) keys on the actual receiver. The resolved |
There was a problem hiding this comment.
Wait what? Did you implement a polymorphic inline cache?
There was a problem hiding this comment.
I can't take credit for this. Several people have worked on this with Robert Muir doing the initial implementation. But yes, we have both this and a MIC :)
| // The handle's parameter shape is now (receiver, [scriptThis], userArgs...). The | ||
| // collectArguments calls below position the lambda filters within the userArgs region, | ||
| // so they need to account for any leading scriptThis slot. | ||
| int handleScriptThisOffset = scriptThisPushed ? 1 : 0; |
There was a problem hiding this comment.
This looks the same as descriptorScriptThisOffset. Are they different?
There was a problem hiding this comment.
Nope, good catch! I will collapse these.
Key the def call-site script-this gate on the user-visible method key (name/arity) instead of the method name alone, so a def call whose argument count can never match a cancellation_aware overload skips the push at compile time. The runtime annotation check in Def.lookupMethod remains the exact decision for genuine collisions.
The descriptor-space and handle-space offsets are always equal: the swap/dropArguments adaptation aligns the resolved handle's shape to the call-site descriptor, so one scriptThisOffset serves both.
Cover the script-this push for cancellation_aware augmentations called inside lambda bodies (static, def, def-encoded, and a loop+each static lambda), and the loopless-lambda entry-time poll that underpins why every lambda captures the script receiver.
The cancellation_aware each augmentation kept its own local poll counter, restarting at the interval and ignoring the script's $cancelPoll progress. Add a generated _pollCancellation() that decrements and checks the shared field, reusing the same helper the compiler emits inline, and call it from the augmentation so polling is amortized across all script work. Drop the dead, local-counter writeCancellationCheck that never honored the field.
Rename the @cancellation_aware annotation to the more general @script_aware (it injects the PainlessScript; cancellation is just the first use). Remove supportsCancellation from the lookup builder: it now always binds the script-first augmentation overload and keeps the annotation, leaving the activation decision to the compiler. The compiler marks script-aware calls as using the script so enclosing lambdas capture it in any context, and the def push is gated only on the script-aware marker. Rename IRCMaybeNeedsScriptThis to IRCScriptAware.
Remove the comments added by this branch from PainlessLookup and PainlessLookupBuilder.
Replace the injectScript boolean with an int offset mirroring augmentedParameterOffset when sizing the java type parameters.
Freeze the inner sets in build() alongside classesToDirectSubClasses so the PainlessLookup constructor can use a plain Map.copyOf.
Have FunctionRef.create detect a @script_aware augmentation delegate and prepend the synthetic script capture itself, exposing isScriptAware so visitFunctionRef just sets the instance-capture condition. Removes the duplicate (and statically-resolved, so always-false) helper. Fixes the capture type to the interface PainlessScript rather than the concrete script class, so LambdaBootstrap rebuilds a delegate descriptor that resolves to the real augmentation method.
Add a regression test where the each consumer is a non-script-aware method ref (no entry poll), so only the each augmentation's own poll can fire cancellation, proving the l::each method reference threads the script into the augmentation.
Remove the comments added by this branch from the four phase classes.
Shorten the IRCScriptAware, IRCCancellationCheck, and IRCStaticCancellationCheck javadocs to one-liners matching the other decoration comments.
In the generated _pollCancellation, dup the cancellation runnable for the null check rather than storing and reloading it; the stored copy still backs the later invoke when the counter reaches zero.
Static-cancellation lambdas skipped the legacy max-loop-counter setup, so a loop had no bound when no cancellation runnable was set at runtime. Set the counter up regardless, matching instance functions, so the loop guard falls back to it when the runnable is null. Also rename IRCCancellationCheck to IRCInstanceCancellationCheck to pair with IRCStaticCancellationCheck.
Match the IRCInstanceCancellationCheck condition and pair with the staticCancellation local.
Remove the writeLoopGuard and writeForEachLoopGuard wrappers and call writeBranchedLoopGuard directly with the legacyForNonOptedIn flag at each loop site.
Rename the writeBranchedLoopGuard parameter legacyForNonOptedIn to legacy.
Load and cast the script instance once and dup it for the getfield/putfield pair instead of loading it twice.
The script slot already holds the generated type at every cancellation field access, so the helper's CHECKCAST was dead. Inline a plain ALOAD and drop the helper.
Relocate the raw $cancelPoll decrement/runnable-invoke bytecode out of DefaultIRTreeToASMBytesPhase and into MethodWriter#writeCancellationPoll, alongside its analogue writeLoopCounter. The phase keeps writeBranchedLoopGuard, which still resolves WriteScope variables and branches, then delegates the raw emission downward. This puts scope/branch logic in the phase and bytecode emission in MethodWriter, matching the existing layering. Also add nested-call regression tests and YAML search-timeout tests for the script-aware augmented each loop.
Fixes a checkstyle LineLength violation (146 > 140) on the IRCStaticCancellationCheck javadoc that failed CI checkPart1. Spotless does not wrap comments, so it passed local formatting but checkstyleMain rejected it.
This change allows us to pass the script instance into augmented methods to allow for cancellation checking.
Adds
@cancellation_awarewhitelist annotation so collection-iterating augmentations (starting withIterable.each) can poll the script's cancel runnable from inside their own driver loop. Trivial lambda bodies that the loop-back-edge counter never reaches (e.g.l.each(x -> x.toString())) now still honour search timeouts.