Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for non-fork-join test executor #3685

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

asardaes
Copy link

Overview

Since there wasn't much feedback in #3108, I figured I'd open a PR with what I have to see if it helps. I reiterate some comments from the linked issue and add some more info.

The idea is to allow a different executor with a fixed thread pool to execute actual tests in parallel in order to avoid potential conflicts with user code if they also rely on the JVM's fork-join APIs. The default would remain as before. Users could then change the executor via configuration.

I wrote a couple of TODOs, but they're both related to my understanding of CONCURRENT execution mode semantics. The current implementation checks execution mode in both submit and forkConcurrentTasks + executeNonConcurrentTasks, but in both scenarios, if the task is not concurrent, it executes it in the current thread, and that doesn't know if some other thread in the pool has stolen work and is executing another test, so is it really guaranteeing that no other test is running at the same time?

I added a method ExclusiveTask#await and use it instead of join in joinConcurrentTasksInReverseOrderToEnableWorkStealing - if the task should execute in a ForkJoinWorkerThread, await delegates to join, otherwise it delegates to compute. If I don't do it like this, the tests execute fine and are even show as PASSED in the terminal, but after that everything stalls and all threads in the fork-join pool remain parked, and I cannot figure out why. I tried to reproduce this outside of a JUnit 5 session and I couldn't.

I added FixedThreadsParallelExecutionIntegrationTests as an almost identical copy of ParallelExecutionIntegrationTests but using the fixed thread pool for test execution, and I can see that there are a lot of failures. I spent some time looking through the logic to see if I could figure out how it's set up, but I couldn't make much progress, I would appreciate feedback if I'm understanding the tests incorrectly. I will point out something more about this in the comments below.


I hereby agree to the terms of the JUnit Contributor License Agreement.


Definition of Done

Comment on lines +127 to +130
ResourceLock resourceLock = testTask.getResourceLock();
if (!(Thread.currentThread() instanceof ForkJoinWorkerThread)) {
resourceLock.release();
}
Copy link
Author

Choose a reason for hiding this comment

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

The canRunTestsIsolatedFromEachOtherWithNestedCases test from FixedThreadsParallelExecutionIntegrationTests was stuck in a deadlock until I added these lines. I spent some time debugging and it seems to me that all of the org.junit.platform.engine.support.hierarchical.SingleLock instances created for the tasks are somehow locked by a fork-join thread before the tasks are scheduled in the fixed thread pool, but no matter how many breakpoints I added, I couldn't figure out where that's happening.

@asardaes asardaes force-pushed the fixed-threads-test-executor branch 2 times, most recently from 335c0cb to f8cb6eb Compare February 10, 2024 15:24
@marcphilipp
Copy link
Member

@asardaes Thank you for your PR! It will be some time before we can get to it but I plan to look into this as part of the work I'm doing for the Sovereign Tech Fund so there is hope. 😉

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

Successfully merging this pull request may close these issues.

JUnit parallel executor running too many tests with a fixed policy.
3 participants