Skip to content

Painless: cancellation_aware augmentation infrastructure - #150092

Merged
jdconrad merged 35 commits into
elastic:mainfrom
jdconrad:painless-cancellation-aware-collections
Jun 3, 2026
Merged

jdconrad merged 35 commits into
elastic:mainfrom
jdconrad:painless-cancellation-aware-collections

Conversation

@jdconrad

@jdconrad jdconrad commented May 28, 2026 •

Copy link
Copy Markdown
Contributor

Adds @cancellation_aware whitelist annotation so collection-iterating augmentations (starting with Iterable.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.

jdconrad added 4 commits May 27, 2026 14:55
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.
jdconrad added 3 commits June 1, 2026 09:53
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.
@jdconrad jdconrad added >enhancement :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache labels Jun 1, 2026
@jdconrad
jdconrad marked this pull request as ready for review June 1, 2026 17:55
@jdconrad
jdconrad requested a review from a team as a code owner June 1, 2026 17:55
@elasticsearchmachine elasticsearchmachine added the Team:Core/Infra Meta label for core/infra team label Jun 1, 2026
@elasticsearchmachine

Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@elasticsearchmachine

Copy link
Copy Markdown
Collaborator

Hi @jdconrad, I've created a changelog YAML for you.

@github-actions

github-actions Bot commented Jun 1, 2026 •

Copy link
Copy Markdown
Contributor

🔍 Preview links for changed docs

⏳ Building and deploying preview... View progress

This comment will be updated with preview links when the build is complete.

@github-actions

github-actions Bot commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

ℹ️ 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 overview

When to use applies_to tags:

✅ At the page level to indicate which products/deployments the content applies to (mandatory)
✅ When features change state (e.g. preview, ga) in a specific version
✅ When availability differs across deployments and environments

What NOT to do:

❌ Don't remove or replace information that applies to an older version
❌ Don't add new information that applies to a specific version without an applies_to tag
❌ Don't forget that applies_to tags can be used at the page, section, and inline level

🤔 Need help?

@prdoyle prdoyle left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't see anything that looks wrong, at least to the extent that I understand what I'm seeing. 😅

Comment on lines +139 to +142
if (cancel == null) {
receiver.forEach(consumer);
return receiver;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a bad idea: call the one the JIT knows about if you can.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +73 to +74
public boolean hasCancellationAwareMethodName(String methodName) {
return cancellationAwareMethodNames.contains(methodName);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I note this is just looking at the method name and is ignoring arity.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +1915 to +1916
methodWriter.loadThis();
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again this seems different from the conditions used to compute pushScriptThis but I can't tell if it's wrong.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as above.

Comment on lines +169 to +179
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);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MethodHandles are fun, aren't they? 😬

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I promise the ordering is correct :)

@prdoyle prdoyle Jun 1, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything in MethodHandles looks backward. It's like talking to Yoda.

Want a handle with fewer arguments? Call insertArguments.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait what? Did you implement a polymorphic inline cache?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks the same as descriptorScriptThisOffset. Are they different?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, good catch! I will collapse these.

jdconrad added 9 commits June 1, 2026 14:02
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.
jdconrad added 16 commits June 2, 2026 10:03
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.
@jdconrad
jdconrad merged commit 4c9f906 into elastic:main Jun 3, 2026
36 checks passed
valeriy42 pushed a commit to valeriy42/elasticsearch that referenced this pull request Jun 18, 2026
This change allows us to pass the script instance into augmented methods to allow for cancellation checking.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Core/Infra/Scripting Scripting abstractions, Painless, and Mustache >enhancement Team:Core/Infra Meta label for core/infra team v9.5.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants