Reduce stack utilization by getting rid of outer Stream API call in TestTemplateTestDescriptor::execute

[amaembo]

Hello, dear JUnit team!

Today I've noticed the following Twitter post which reports a very deep stack trace:
https://x.com/gunnarmorling/status/1838556082005807554

It includes a lot of JUnit frames, notably coming from the nested Stream API call in TestTemplateTestDescriptor#execute. I've noticed that a short and local patch could reduce the stack consumption by probably a couple dozens of frames without changing the code behavior:

diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/TestTemplateTestDescriptor.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/TestTemplateTestDescriptor.java
--- a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/TestTemplateTestDescriptor.java	(revision 926e31f87334ababc45bd31b0accdb54f909041b)
+++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/TestTemplateTestDescriptor.java	(date 1727192184035)
@@ -101,12 +101,11 @@
 			context.getExtensionRegistry());
 		AtomicInteger invocationIndex = new AtomicInteger();
 		// @formatter:off
-		providers.stream()
-				.flatMap(provider -> provider.provideTestTemplateInvocationContexts(extensionContext))
-				.map(invocationContext -> createInvocationTestDescriptor(invocationContext, invocationIndex.incrementAndGet()))
-				.filter(Optional::isPresent)
-				.map(Optional::get)
-				.forEach(invocationTestDescriptor -> execute(dynamicTestExecutor, invocationTestDescriptor));
+		for (TestTemplateInvocationContextProvider provider : providers) {
+			provider.provideTestTemplateInvocationContexts(extensionContext)
+				.forEach(invocationContext -> createInvocationTestDescriptor(invocationContext, invocationIndex.incrementAndGet())
+					.ifPresent(invocationTestDescriptor -> execute(dynamicTestExecutor, invocationTestDescriptor)));
+		}
 		// @formatter:on
 		validateWasAtLeastInvokedOnce(invocationIndex.get(), providers);
 		return context;

Reducing stack consumption makes the debugging experience more user-friendly, as users will be less likely confused by many frames. It may also reduce the output size if the exception is printed/logged, saving the precious screen or log file real estate and time necessary to scroll through the output. Finally, using less stack size by framework leaves more stack size to the user code, which is also user-friendly. While streams are used in other places, this one is particularly important because in the heart of it the user code is executed, so users can see this particular stack trace quite often.

A further stack consumption reduction is possible, though it requires collecting streams from provideTestTemplateInvocationContexts to intermediate lists, so it could be more questionable, as performance and memory consumption are not definitely better in this case:

diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/TestTemplateTestDescriptor.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/TestTemplateTestDescriptor.java
--- a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/TestTemplateTestDescriptor.java	(revision 926e31f87334ababc45bd31b0accdb54f909041b)
+++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/descriptor/TestTemplateTestDescriptor.java	(date 1727192184035)
@@ -101,12 +101,13 @@
 			context.getExtensionRegistry());
 		AtomicInteger invocationIndex = new AtomicInteger();
 		// @formatter:off
-		providers.stream()
-				.flatMap(provider -> provider.provideTestTemplateInvocationContexts(extensionContext))
-				.map(invocationContext -> createInvocationTestDescriptor(invocationContext, invocationIndex.incrementAndGet()))
-				.filter(Optional::isPresent)
-				.map(Optional::get)
-				.forEach(invocationTestDescriptor -> execute(dynamicTestExecutor, invocationTestDescriptor));
+		for (TestTemplateInvocationContextProvider provider : providers) {
+			for (TestTemplateInvocationContext invocationContext : provider.provideTestTemplateInvocationContexts(
+					extensionContext).collect(toList())) {
+				createInvocationTestDescriptor(invocationContext, invocationIndex.incrementAndGet()).ifPresent(
+						invocationTestDescriptor -> execute(dynamicTestExecutor, invocationTestDescriptor));
+			}
+		}
 		// @formatter:on
 		validateWasAtLeastInvokedOnce(invocationIndex.get(), providers);
 		return context;

It's just a suggestion, feel free to ignore if it goes against the code standards in the JUnit project.

[sormuras]

If only @gunnarmorling ran JUnit on the module path, I could see the version he was using. ;-)

Since 1.10, we try our best to prune such stacktraces: https://junit.org/junit5/docs/current/user-guide/#stacktrace-pruning

[amaembo]

@sormuras I believe the screenshot is taken from a debugging session (I recognize the Eclipse UI). I think the pruning doesn't affect the debugger behavior?

[sormuras]

That's true, stackframes in debugging sessions are not pruned.

[gunnarmorling]

Yes, indeed this is with the Eclipse debugger.

[sormuras]

Here's the screenshot we're talking about.

[marcphilipp]

It includes a lot of JUnit frames, notably coming from the nested Stream API call in TestTemplateTestDescriptor#execute. I've noticed that a short and local patch could reduce the stack consumption by probably a couple dozens of frames without changing the code behavior

I think that's a reasonable refactoring. 👍

A further stack consumption reduction is possible, though it requires collecting streams from provideTestTemplateInvocationContexts to intermediate lists, so it could be more questionable, as performance and memory consumption are not definitely better in this case

We should indeed not change this as lazily processing TestTemplateInvocationContext is a feature when it comes to parameterizing tests based in files or memory-intensive resources.

@amaembo Would you be interested in sending a PR for the first suggested change?

[amaembo]

Sure, here it is: #4024
This is my first contribution to this project, hopefully done correctly :-)

[amaembo]

I see the problem with closeable stream. Will address it.

[sormuras]

Note that IntelliJ IDEA has an option to hide frames within an active debug session.

[sormuras]

Related low-level idea: https://bugs.openjdk.org/browse/JDK-8340973