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

Surprisingly limited parallel test execution with ResourceLock and ResourceAccessMode.READ #2038

Closed
1 task
bmalinowsky opened this issue Oct 3, 2019 · 6 comments

Comments

@bmalinowsky
Copy link

bmalinowsky commented Oct 3, 2019

I observed the following behavior as detailed below, where test methods using resource lock with READ mode appear to be unnecessarily sequenced.

Even though ResourceLock documents that [...] the annotated element _may_ be executed concurrently with other test classes or methods [...] (emphasis mine), the observed test executions are somewhat a surprise to me.

Steps to reproduce

Consider the following MWE:

import static org.junit.jupiter.api.Assertions.assertEquals;

import java.util.concurrent.atomic.AtomicInteger;
import java.util.stream.IntStream;

import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;


//@ResourceLock(value = "sharedResource", mode = ResourceAccessMode.READ)
class ResourceLockTest {
	private static AtomicInteger executingRead = new AtomicInteger();
	private static AtomicInteger maxRead = new AtomicInteger();

	@AfterAll
	static void afterAll() { System.out.println("max executing reads = " + maxRead); }

	@ParameterizedTest
	@MethodSource("ints")
//	@ResourceLock(value = "sharedResource", mode = ResourceAccessMode.READ)
	void test(final int arg) { rememberMax(); }

	@Test
//	@ResourceLock(value = "sharedResource", mode = ResourceAccessMode.READ)
	void test2() { rememberMax(); }

	@Test
//	@ResourceLock(value = "sharedResource", mode = ResourceAccessMode.READ)
	void test3() { rememberMax(); }

	@Test
//	@ResourceLock(value = "sharedResource", mode = ResourceAccessMode.READ)
	void test4() { rememberMax(); }

	@Test
//	@ResourceLock(value = "sharedResource", mode = ResourceAccessMode.READ_WRITE)
	void testWrite() { assertEquals(0, executingRead.get()); }

	private void rememberMax() {
		final int active = executingRead.incrementAndGet();
		maxRead.getAndUpdate(v -> v > active ? v : active);
		try {
			Thread.sleep(50);
		}
		catch (final InterruptedException e) {}
		executingRead.decrementAndGet();
	}

	private static IntStream ints() { return IntStream.rangeClosed(1, 100); }
}

Results

  • Baseline to check correct junit concurrency config, running with resource lock commented out: consistently results in max executing reads of 8 (on my machine), and testWrite can fail. Expected.

  • Enable ResourceLocks on methods:

    • normal test methods with READ run usually in parallel. Expected.
    • the parameterized test has consistently a max read of 1. Correct, but unexpected.
  • Enable ResourceLock on class: consistently a max read of 1. Correct, but very unexpected.

Especially the last point I find surprising. With many test methods, it makes sense to move the lock to the class (and not have it on each method). Such change seems valid but results in degraded execution time.

Context

junitJupiterVersion = '5.5.2'

Gradle 5.6.2
Build time:   2019-09-05 16:13:54 UTC
Revision:     55a5e53d855db8fc7b0e494412fc624051a8e781

Kotlin:       1.3.41
Groovy:       2.5.4
Ant:          Apache Ant(TM) version 1.9.14 compiled on March 12 2019
JVM:          12.0.2-BellSoft (BellSoft 12.0.2-BellSoft+11)
OS:           Mac OS X 10.15 x86_64

junit.platform.properties

junit.jupiter.execution.parallel.enabled = true
junit.jupiter.execution.parallel.mode.default = concurrent
junit.jupiter.execution.parallel.mode.classes.default = concurrent
junit.jupiter.execution.parallel.config.dynamic.factor = 1

Deliverables

  • Clarify if the current performance behavior of the built-in ResourceLock is intended
@marcphilipp
Copy link
Member

The (pessimistic) assumption behind forcing children of containers with resource locks to use execution mode SAME_THREAD is that the resource might not be safe to access in e.g. @BeforeAll methods and in different threads from @Test methods. Thus it's not the same as having @ResourceLock on every @Test method. IIRC we had a similar issue for @Execution only applying to methods while being declared on the class level.

@marcphilipp
Copy link
Member

Tentatively slated for 5.6 M2 for team discussion.

@marcphilipp marcphilipp added this to the 5.7 M1 milestone Apr 4, 2020
@marcphilipp marcphilipp modified the milestones: 5.7 M1, 5.7 M2 Apr 17, 2020
@marcphilipp marcphilipp modified the milestones: 5.7 M2, 5.7 Backlog May 29, 2020
@leonard84
Copy link
Collaborator

I had discussed with @marcphilipp that there should be something like @ResourceLockChildren that only applies to the children of a TestDescriptor but not the descriptor itself, i.e. @ResourceLockChildren on the test class would be the same as applying @ResourceLock on each test method.

@stale
Copy link

stale bot commented Jun 19, 2022

This issue has been automatically marked as stale because it has not had recent activity. Given the limited bandwidth of the team, it will be automatically closed if no further activity occurs. Thank you for your contribution.

@stale stale bot added the status: stale label Jun 19, 2022
@leonard84
Copy link
Collaborator

@marcphilipp I thin this can be closed as it has been fixed by my PR AFAIK.

@stale stale bot removed the status: stale label Jun 22, 2022
@marcphilipp
Copy link
Member

Duplicate of #2423

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

No branches or pull requests

4 participants