Skip to content

Replace serialization-based type inference with reified varargs type token#1524

Closed
mdproctor wants to merge 5 commits into
open-workflow-specification:mainfrom
mdproctor:refactor/reified-varargs-type-token
Closed

Replace serialization-based type inference with reified varargs type token#1524
mdproctor wants to merge 5 commits into
open-workflow-specification:mainfrom
mdproctor:refactor/reified-varargs-type-token

Conversation

@mdproctor

Copy link
Copy Markdown
Contributor

Summary

  • Replace SerializableFunction/SerializablePredicate/SerializableConsumer + ReflectionUtils.inferInputType() with the reified varargs type token pattern across ~40 call sites
  • Delete ReflectionUtils, SerializableFunction, SerializablePredicate, SerializableConsumer — no longer needed
  • Drop Serializable from InstanceIdFunction and UniqueIdBiFunction (was only needed for the inference hack)
  • Add test demonstrating nested generic type safety (Map<String, List<Integer>>)

Motivation

The previous approach used JVM serialization internals (writeReplaceSerializedLambdagetInstantiatedMethodType) to recover erased type parameters at runtime. This is:

  • Fragile — depends on undocumented JVM serialization behavior
  • Restrictive — forces all lambdas to implement Serializable
  • Heavyweight — serialization roundtrip + reflection per inference call

The reified varargs type token pattern achieves the same result (recovering the raw Class<T> at runtime) with zero reflection, zero serialization, and works with any lambda or method reference regardless of serializability. Java reifies array creation at the call site even for generic varargs parameters, so T... typeToken with zero arguments creates a new T[0] whose component type is recoverable via getClass().getComponentType().

Test plan

  • All existing tests pass (0 regressions)
  • New test: function_with_nested_generics_preserves_type_safety — verifies Map<String, List<Integer>> flows through with compile-time type safety
  • IntelliJ build: 0 errors

🤖 Generated with Claude Code

@mdproctor
mdproctor requested a review from fjtirado as a code owner July 8, 2026 19:18
Copilot AI review requested due to automatic review settings July 8, 2026 19:18

Copilot AI 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.

Pull request overview

This PR removes the previous serialization/SerializedLambda-based runtime type inference utilities and migrates the fluent function DSL to a “reified varargs type token” approach for recovering the raw input Class<T> at call sites, eliminating the need for Serializable* functional interfaces and ReflectionUtils.

Changes:

  • Deleted ReflectionUtils and the SerializableFunction/SerializablePredicate/SerializableConsumer helper interfaces; removed Serializable from InstanceIdFunction and UniqueIdBiFunction.
  • Updated fluent DSL entrypoints (FuncDSL, Step, CommonFuncOps, and related specs/builders) to accept standard JDK functional interfaces plus an optional varargs type token for input class inference.
  • Added a test intended to demonstrate nested generic lambda type safety.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
experimental/types/src/main/java/io/serverlessworkflow/api/reflection/func/UniqueIdBiFunction.java Drops Serializable from the functional interface now that serialization inference is removed.
experimental/types/src/main/java/io/serverlessworkflow/api/reflection/func/SerializablePredicate.java Removed obsolete serializable predicate shim.
experimental/types/src/main/java/io/serverlessworkflow/api/reflection/func/SerializableFunction.java Removed obsolete serializable function shim.
experimental/types/src/main/java/io/serverlessworkflow/api/reflection/func/SerializableConsumer.java Removed obsolete serializable consumer shim.
experimental/types/src/main/java/io/serverlessworkflow/api/reflection/func/ReflectionUtils.java Deleted serialization/reflection-based lambda type inference utility.
experimental/types/src/main/java/io/serverlessworkflow/api/reflection/func/InstanceIdFunction.java Drops Serializable from the functional interface now that serialization inference is removed.
experimental/fluent/func/src/test/java/io/serverlessworkflow/fluent/func/FuncDSLTest.java Adds a nested-generics-focused test for the updated DSL typing behavior.
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/FuncEmitEventPropertiesBuilder.java Accepts Function directly instead of a serializable wrapper.
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/Step.java Replaces reflection-based inference with varargs type token input-class derivation for chained operations.
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/FuncEventFilterSpec.java Switches envelope/data filter overloads from serializable predicates to standard Predicate.
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/FuncEmitSpec.java Adds varargs type-token overload for JSON event data emission.
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/FuncDSL.java Updates core DSL helpers to use varargs type tokens for input class inference and removes reflection-based return-type inference wiring.
experimental/fluent/func/src/main/java/io/serverlessworkflow/fluent/func/dsl/CommonFuncOps.java Updates shared DSL ops to accept standard Function/Predicate with type-token inference.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +687 to +690
List<TaskItem> items = wf.getDo();
assertEquals(1, items.size());
assertNotNull(items.get(0).getTask().getCallTask(), "CallTask expected");
}
Copilot AI review requested due to automatic review settings July 9, 2026 08:52

Copilot AI 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.

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.

Comment on lines +32 to +35
@SuppressWarnings("unchecked")
private static <R> Class<R> defaultReturnClass(Class<R> c) {
return c != null ? c : (Class<R>) Object.class;
}
Comment on lines +687 to +690
List<TaskItem> items = wf.getDo();
assertEquals(1, items.size());
assertNotNull(items.get(0).getTask().getCallTask(), "CallTask expected");
}
Copilot AI review requested due to automatic review settings July 9, 2026 09:24

Copilot AI 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.

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.

Comment on lines +34 to +37
@SuppressWarnings("unchecked")
private static <R> Class<R> defaultReturnClass(Class<R> c) {
return c != null ? c : (Class<R>) Object.class;
}
this.filterFn = null;
this.argClass = argClass;
this.returnClass = returnClass;
this.returnClass = defaultReturnClass(returnClass);
this.filterFn = null;
this.argClass = argClass;
this.returnClass = returnClass;
this.returnClass = defaultReturnClass(returnClass);
this.filterFn = filterFn;
this.argClass = argClass;
this.returnClass = returnClass;
this.returnClass = defaultReturnClass(returnClass);
mdproctor and others added 3 commits July 9, 2026 10:41
…token

The fluent DSL previously used SerializableFunction/Predicate/Consumer interfaces
and ReflectionUtils.inferInputType() to recover generic type information at runtime.
This relied on JVM serialization internals (writeReplace → SerializedLambda) which
are fragile, heavyweight, and force all lambdas to implement Serializable.

Replace with the reified varargs type token pattern: declaring a trailing T... parameter
causes Java to create a reified array at the call site, making the erased-but-concrete
class recoverable via getClass().getComponentType(). This gives the same runtime
Class<T> with zero reflection, zero serialization, zero allocation, and works with
any lambda or method reference regardless of serializability.

Changes:
- Replace ~40 call sites across FuncDSL, Step, CommonFuncOps, FuncEmitSpec,
  FuncEmitEventPropertiesBuilder, and FuncEventFilterSpec
- Delete ReflectionUtils, SerializableFunction, SerializablePredicate,
  SerializableConsumer (no longer needed)
- Drop Serializable from InstanceIdFunction and UniqueIdBiFunction (was only
  needed for the inference hack; ContextFunction and FilterFunction keep it
  as they are stored in serializable CallJava objects)
- Add test demonstrating nested generic type safety (Map<String, List<Integer>>)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Mark Proctor <mdproctor@gmail.com>
AbstractJavaCallExecutor uses outputClass.isPresent() to decide whether
to run functions via CompletableFuture.supplyAsync (async) or
completedFuture (sync). Passing null for the return class caused
functions to run synchronously, breaking the async contract that
FuncCallAsyncTest.testReferencedFunctionCall relies on.

Default null returnClass to Object.class in FuncCallStep constructors
so the executor always takes the async path.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Mark Proctor <mdproctor@gmail.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Mark Proctor <mdproctor@gmail.com>
@mdproctor
mdproctor force-pushed the refactor/reified-varargs-type-token branch from a64f8bc to 2e76c0c Compare July 9, 2026 09:42
The executor's else branch (null outputClass) ran the function
synchronously on the calling thread, unlike the outputClass.isPresent()
branch which used supplyAsync. This caused timing-sensitive tests to
fail when the varargs refactoring stopped providing an output class.

Change the else branch to also use supplyAsync, then thenCompose with
the runtime instanceof CompletableFuture check. This correctly handles
both sync and async function return types while always executing on
the background thread pool.

Revert the Object.class default in FuncCallStep — null outputClass is
now handled correctly by the executor.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Mark Proctor <mdproctor@gmail.com>
Copilot AI review requested due to automatic review settings July 10, 2026 02:33

Copilot AI 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.

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 3 comments.

Comment on lines +687 to +690
List<TaskItem> items = wf.getDo();
assertEquals(1, items.size());
assertNotNull(items.get(0).getTask().getCallTask(), "CallTask expected");
}
Comment on lines 32 to 36
@SuppressWarnings("unchecked")
default <T, V> Consumer<FuncCallTaskBuilder> fn(Function<T, V> function, T... typeToken) {
Class<T> clazz = (Class<T>) typeToken.getClass().getComponentType();
return fn(function, clazz);
}
Comment on lines 50 to 53
@SuppressWarnings("unchecked")
default <T> SwitchCaseSpec<T> caseOf(Predicate<T> when, T... typeToken) {
return caseOf(when, (Class<T>) typeToken.getClass().getComponentType());
}
@fjtirado

fjtirado commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

@mdproctor

We need serializable functions, consumer and predicates for YAML/JSON serialization, so the serialization classes cannot be removed. They must be kept for Json/yaml serialization regardless of the inference mechanism used.

For type inference, the PR proposal is smarter and faster than the current inference method based on SerializedLambda. But there is a caveat, it does not resolve the output class, which is internally used during execution time to convert the output. I'm not sure the solution taken in AbstractJavaCallExecutor to circumvent that is correct ( I guess the CI will fail). And even if it does not fail, the consequence of not knowing in advance that the function is returning a completable future is that we were creating an additional async request unnecessarily, which is probably heavier (I elaborate more in the last paragraph) than accessing the serializedlamda to check if the function is returning a CompletableFuture.

That's said, I really liked the approach for input parameter taken here, and since we need SerializedLambda for YAML/Json serialization, I'm thinking on an hybrid approach. I was already contemplating some extra improvements on ReflectionUtils (calling inferMethodType only once and cache the resolution of Method Type) which, together with the component type approach might dramatically improve type inference performance without losing the completablefuture check at runtime.

An important remark about inference performance, the type inference only occurs during workflow definition time (typically at startup), so the performance improvement does not really affect execution time, but startup time, since the definitions are loaded at startup, hence we do not invest much effort on improving type inference performance (in other words, type inference is a one time operation, that is the reason why the CompletablueFuture change is important, it affects all workflow executions, while output class inference is just performed once during workflow definition, so it can be argued that the current change in AbstractJavaCallExceutor will make performance worse when invoking java methods that are already async, as paradoxically as it might sound)

…ssertion

- Add "varargs" to @SuppressWarnings on CommonFuncOps default methods
- Strengthen nested generics test to verify inferred inputClass is Map.class

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Signed-off-by: Mark Proctor <mdproctor@gmail.com>
Copilot AI review requested due to automatic review settings July 10, 2026 18:08

Copilot AI 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.

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.

Comment on lines +950 to 952
public static <T> EmitStep emit(String type, Function<T, CloudEventData> fn) {
return new EmitStep(null, produced(type, fn));
}
Comment on lines +963 to 965
public static <T> EmitStep emit(String name, String type, Function<T, CloudEventData> fn) {
return new EmitStep(name, produced(type, fn));
}
@ricardozanini

Copy link
Copy Markdown
Collaborator

PR for reference only, thanks Mark!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants