Skip to content

Commit

Permalink
Allow parallel execution of child nodes if only read locks are acquir…
Browse files Browse the repository at this point in the history 
…ed (#2515)
  • Loading branch information
@leonard84
leonard84 committed Jan 12, 2021
1 parent 02a3e1f commit a58ef50
Show file tree
Hide file tree
Showing 3 changed files with 128 additions and 9 deletions.
3 changes: 2 additions & 1 deletion documentation/src/docs/asciidoc/release-notes/release-notes-5.8.0-M1.adoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ on GitHub.
* Documented constant value of `ExclusiveResource.GLOBAL_KEY`.
* Instances of `TestIdentifier` and `UniqueId` now retain less memory because they no
longer store `String` representations of unique IDs.

* Improvement of `ExclusiveResource` handling, if a `Node` has only read locks and no read-write locks,
then descendants are not forced into `SAME_THREAD` execution, and can run concurrently.

[[release-notes-5.8.0-M1-junit-jupiter]]
=== JUnit Jupiter
Expand Down
34 changes: 26 additions & 8 deletions ...m-engine/src/main/java/org/junit/platform/engine/support/hierarchical/NodeTreeWalker.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,21 +57,39 @@ private void walk(TestDescriptor globalLockDescriptor, TestDescriptor testDescri
}
else {
Set<ExclusiveResource> allResources = new HashSet<>(exclusiveResources);
advisor.forceDescendantExecutionMode(testDescriptor, SAME_THREAD);
doForChildrenRecursively(testDescriptor, child -> {
allResources.addAll(getExclusiveResources(child));
advisor.forceDescendantExecutionMode(child, SAME_THREAD);
});
if (isReadOnly(allResources)) {
doForChildrenRecursively(testDescriptor, child -> allResources.addAll(getExclusiveResources(child)));
if (!isReadOnly(allResources)) {
forceDescendantExecutionModeRecursively(advisor, testDescriptor);
}
}
else {
advisor.forceDescendantExecutionMode(testDescriptor, SAME_THREAD);
doForChildrenRecursively(testDescriptor, child -> {
allResources.addAll(getExclusiveResources(child));
advisor.forceDescendantExecutionMode(child, SAME_THREAD);
});
}
if (!globalLockDescriptor.equals(testDescriptor) && allResources.contains(GLOBAL_READ_WRITE)) {
advisor.forceDescendantExecutionMode(globalLockDescriptor, SAME_THREAD);
doForChildrenRecursively(globalLockDescriptor,
child -> advisor.forceDescendantExecutionMode(child, SAME_THREAD));
forceDescendantExecutionModeRecursively(advisor, globalLockDescriptor);
advisor.useResourceLock(globalLockDescriptor, globalReadWriteLock);
}
advisor.useResourceLock(testDescriptor, lockManager.getLockForResources(allResources));
}
}

private void forceDescendantExecutionModeRecursively(NodeExecutionAdvisor advisor,
TestDescriptor globalLockDescriptor) {
advisor.forceDescendantExecutionMode(globalLockDescriptor, SAME_THREAD);
doForChildrenRecursively(globalLockDescriptor,
child -> advisor.forceDescendantExecutionMode(child, SAME_THREAD));
}

private boolean isReadOnly(Set<ExclusiveResource> exclusiveResources) {
return exclusiveResources.stream().allMatch(
exclusiveResource -> exclusiveResource.getLockMode() == ExclusiveResource.LockMode.READ);
}

private Set<ExclusiveResource> getExclusiveResources(TestDescriptor testDescriptor) {
return NodeUtils.asNode(testDescriptor).getExclusiveResources();
}
Expand Down
100 changes: 100 additions & 0 deletions ...t/java/org/junit/platform/engine/support/hierarchical/NodeTreeWalkerIntegrationTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import static org.junit.platform.engine.discovery.DiscoverySelectors.selectClass;
import static org.junit.platform.engine.support.hierarchical.ExclusiveResource.GLOBAL_READ;
import static org.junit.platform.engine.support.hierarchical.ExclusiveResource.GLOBAL_READ_WRITE;
import static org.junit.platform.engine.support.hierarchical.ExclusiveResource.LockMode.READ;
import static org.junit.platform.engine.support.hierarchical.ExclusiveResource.LockMode.READ_WRITE;
import static org.junit.platform.engine.support.hierarchical.Node.ExecutionMode.SAME_THREAD;
import static org.junit.platform.launcher.core.LauncherDiscoveryRequestBuilder.request;
Expand All @@ -25,6 +26,7 @@

import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.parallel.ResourceAccessMode;
import org.junit.jupiter.api.parallel.ResourceLock;
import org.junit.jupiter.engine.JupiterTestEngine;
import org.junit.platform.engine.TestDescriptor;
Expand Down Expand Up @@ -54,6 +56,70 @@ void pullUpExclusiveChildResourcesToTestClass() {
assertThat(advisor.getForcedExecutionMode(testMethodDescriptor)).contains(SAME_THREAD);
}

@Test
void setsForceExecutionModeForChildrenWithWriteLocksOnClass() {
var engineDescriptor = discover(TestCaseWithResourceWriteLockOnClass.class);

var advisor = nodeTreeWalker.walk(engineDescriptor);

var testClassDescriptor = getOnlyElement(engineDescriptor.getChildren());
assertThat(advisor.getResourceLock(testClassDescriptor)).extracting(allLocks()) //
.isEqualTo(List.of(getReadWriteLock("a")));
assertThat(advisor.getForcedExecutionMode(testClassDescriptor)).isEmpty();

var testMethodDescriptor = getOnlyElement(testClassDescriptor.getChildren());
assertThat(advisor.getResourceLock(testMethodDescriptor)).extracting(allLocks()).isEqualTo(List.of());
assertThat(advisor.getForcedExecutionMode(testMethodDescriptor)).contains(SAME_THREAD);
}

@Test
void doesntSetForceExecutionModeForChildrenWithReadLocksOnClass() {
var engineDescriptor = discover(TestCaseWithResourceReadLockOnClass.class);

var advisor = nodeTreeWalker.walk(engineDescriptor);

var testClassDescriptor = getOnlyElement(engineDescriptor.getChildren());
assertThat(advisor.getResourceLock(testClassDescriptor)).extracting(allLocks()) //
.isEqualTo(List.of(getReadLock("a")));
assertThat(advisor.getForcedExecutionMode(testClassDescriptor)).isEmpty();

var testMethodDescriptor = getOnlyElement(testClassDescriptor.getChildren());
assertThat(advisor.getResourceLock(testMethodDescriptor)).extracting(allLocks()).isEqualTo(List.of());
assertThat(advisor.getForcedExecutionMode(testMethodDescriptor)).isEmpty();
}

@Test
void setsForceExecutionModeForChildrenWithReadLocksOnClassAndWriteLockOnTest() {
var engineDescriptor = discover(TestCaseWithResourceReadLockOnClassAndWriteClockOnTest.class);

var advisor = nodeTreeWalker.walk(engineDescriptor);

var testClassDescriptor = getOnlyElement(engineDescriptor.getChildren());
assertThat(advisor.getResourceLock(testClassDescriptor)).extracting(allLocks()) //
.isEqualTo(List.of(getReadWriteLock("a")));
assertThat(advisor.getForcedExecutionMode(testClassDescriptor)).isEmpty();

var testMethodDescriptor = getOnlyElement(testClassDescriptor.getChildren());
assertThat(advisor.getResourceLock(testMethodDescriptor)).extracting(allLocks()).isEqualTo(List.of());
assertThat(advisor.getForcedExecutionMode(testMethodDescriptor)).contains(SAME_THREAD);
}

@Test
void doesntSetForceExecutionModeForChildrenWithReadLocksOnClassAndReadLockOnTest() {
var engineDescriptor = discover(TestCaseWithResourceReadLockOnClassAndReadClockOnTest.class);

var advisor = nodeTreeWalker.walk(engineDescriptor);

var testClassDescriptor = getOnlyElement(engineDescriptor.getChildren());
assertThat(advisor.getResourceLock(testClassDescriptor)).extracting(allLocks()) //
.isEqualTo(List.of(getReadLock("a"), getReadLock("b")));
assertThat(advisor.getForcedExecutionMode(testClassDescriptor)).isEmpty();

var testMethodDescriptor = getOnlyElement(testClassDescriptor.getChildren());
assertThat(advisor.getResourceLock(testMethodDescriptor)).extracting(allLocks()).isEqualTo(List.of());
assertThat(advisor.getForcedExecutionMode(testMethodDescriptor)).isEmpty();
}

@Test
void leavesResourceLockOnTestMethodWhenClassDoesNotUseResource() {
var engineDescriptor = discover(TestCaseWithoutResourceLock.class);
Expand Down Expand Up @@ -112,6 +178,10 @@ private Lock getReadWriteLock(String key) {
return getLock(new ExclusiveResource(key, READ_WRITE));
}

private Lock getReadLock(String key) {
return getLock(new ExclusiveResource(key, READ));
}

private Lock getLock(ExclusiveResource exclusiveResource) {
return getOnlyElement(ResourceLockSupport.getLocks(lockManager.getLockForResource(exclusiveResource)));
}
Expand Down Expand Up @@ -154,4 +224,34 @@ void test() {
}
}
}

@ResourceLock("a")
static class TestCaseWithResourceWriteLockOnClass {
@Test
void test() {
}
}

@ResourceLock(value = "a", mode = ResourceAccessMode.READ)
static class TestCaseWithResourceReadLockOnClass {
@Test
void test() {
}
}

@ResourceLock(value = "a", mode = ResourceAccessMode.READ)
static class TestCaseWithResourceReadLockOnClassAndWriteClockOnTest {
@Test
@ResourceLock("a")
void test() {
}
}

@ResourceLock(value = "a", mode = ResourceAccessMode.READ)
static class TestCaseWithResourceReadLockOnClassAndReadClockOnTest {
@Test
@ResourceLock(value = "b", mode = ResourceAccessMode.READ)
void test() {
}
}
}

0 comments on commit a58ef50

Please sign in to comment.