forked from spockframework/spock
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Use multiple locks in ByteBuddyMockFactory to reduce lock contention
Changed locking scheme in ByteBuddyMockFactory from a single global lock CACHE to cacheLocks with size 16. The used TypeCachingLock from cacheLocks depends on the hashcode of TypeCache.SimpleKey, which aggregates the types and settings to mock. The old global CACHE lock did block Threads regardless, if they try to mock the same type or not. This is a similar fix as in Mockito: mockito/mockito#3095
- Loading branch information
Showing
3 changed files
with
275 additions
and
3 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
146 changes: 146 additions & 0 deletions
146
...src/test/groovy/org/spockframework/mock/runtime/ByteBuddyMockFactoryConcurrentSpec.groovy
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,146 @@ | ||
| /* | ||
| * Copyright 2023 the original author or authors. | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * https://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package org.spockframework.mock.runtime | ||
|
|
||
| import groovy.transform.Canonical | ||
| import spock.lang.Isolated | ||
| import spock.lang.Shared | ||
| import spock.lang.Specification | ||
| import spock.lang.Timeout | ||
| import spock.lang.Unroll | ||
|
|
||
| import java.util.concurrent.CompletableFuture | ||
| import java.util.concurrent.Phaser | ||
| import java.util.concurrent.TimeUnit | ||
| import java.util.function.Function | ||
|
|
||
| // We need to run in isolation, because the ByteBuddyMockFactory is static and we interfere with the cache | ||
| // In the future we should change the ByteBuddyMockFactory to be an instance instead of static API. | ||
| @Isolated | ||
| class ByteBuddyMockFactoryConcurrentSpec extends Specification { | ||
| private static final String IfA = "IfA" | ||
| private static final String IfB = "IfB" | ||
| private static final String IfC = "IfC" | ||
| @Shared | ||
| final IProxyBasedMockInterceptor interceptor = Stub() | ||
|
|
||
| // Just to be save to abort, normally the tests run in 2 secs. | ||
| @Timeout(40) | ||
| @Unroll("#test") | ||
| def "cacheLockingStressTest"(String test, MockSpec mockSpecA, MockSpec mockSpecB) { | ||
| setup: | ||
| int iterations = 5_000 | ||
| def tempClassLoader = new ByteBuddyTestClassLoader() | ||
| MockFeatures featA = toMockFeatures(mockSpecA, tempClassLoader) | ||
| MockFeatures featB = toMockFeatures(mockSpecB, tempClassLoader) | ||
|
|
||
| Phaser phaser = new Phaser(4) | ||
| Function<Runnable, CompletableFuture<Void>> runCode = { Runnable code -> | ||
| CompletableFuture.runAsync({ | ||
| phaser.arriveAndAwaitAdvance() | ||
| try { | ||
| for (int i = 0; i < iterations; i++) { | ||
| code.run() | ||
| } | ||
| } finally { | ||
| phaser.arrive() | ||
| } | ||
| }) | ||
| } | ||
| when: | ||
| def mockFeatAFuture = runCode.apply { | ||
| Class<?> mockClass = mockClass(tempClassLoader, featA) | ||
| assertValidMockClass(featA, mockClass, tempClassLoader) | ||
| } | ||
|
|
||
| def mockFeatBFuture = runCode.apply { | ||
| Class<?> mockClass = mockClass(tempClassLoader, featB) | ||
| assertValidMockClass(featB, mockClass, tempClassLoader) | ||
| } | ||
|
|
||
| def cacheFuture = runCode.apply(ByteBuddyMockFactory::clearAllCaches) | ||
|
|
||
| phaser.arriveAndAwaitAdvance() | ||
| // Wait for test to end | ||
| int phase = phaser.arrive() | ||
| try { | ||
| phaser.awaitAdvanceInterruptibly(phase, 30, TimeUnit.SECONDS) | ||
| } finally { | ||
| // Collect exceptions from the futures, to make issues visible. | ||
| mockFeatAFuture.getNow(null) | ||
| mockFeatBFuture.getNow(null) | ||
| cacheFuture.getNow(null) | ||
| } | ||
| then: | ||
| noExceptionThrown() | ||
|
|
||
| where: | ||
| test | mockSpecA | mockSpecB | ||
| "same hashcode different mockType" | mockSpec(IfA, IfB) | mockSpec(IfB, IfA) | ||
| "same hashcode same mockType" | mockSpec(IfA) | mockSpec(IfA) | ||
| "different hashcode different interfaces" | mockSpec(IfA, IfB) | mockSpec(IfB, IfC) | ||
| "unrelated classes" | mockSpec(IfA) | mockSpec(IfB) | ||
| } | ||
|
|
||
| private Class<?> mockClass(ClassLoader cl, MockFeatures feature) { | ||
| return ByteBuddyMockFactory.createMock( | ||
| feature.mockType, | ||
| feature.interfaces, | ||
| null, | ||
| interceptor, | ||
| cl, | ||
| false) | ||
| .getClass() | ||
| } | ||
|
|
||
| private static MockSpec mockSpec(String mockedType, String... interfaces) { | ||
| return new MockSpec(mockedType, Arrays.asList(interfaces)) | ||
| } | ||
|
|
||
| private void assertValidMockClass(MockFeatures mockFeature, Class<?> mockClass, ClassLoader classLoader) { | ||
| assert mockClass.classLoader == classLoader | ||
| assert mockFeature.mockType.isAssignableFrom(mockClass) | ||
| for (Class<?> anInterface : mockFeature.interfaces) { | ||
| assert anInterface.isAssignableFrom(mockClass) | ||
| } | ||
| } | ||
|
|
||
| MockFeatures toMockFeatures(MockSpec mockFeaturesString, ByteBuddyTestClassLoader classLoader) { | ||
| def mockType = classLoader.defineInterface(mockFeaturesString.mockType) | ||
| def interfaces = mockFeaturesString.interfaces.collect { classLoader.defineInterface(it) } | ||
| return new MockFeatures(mockType, interfaces) | ||
| } | ||
|
|
||
| /** | ||
| * Class holding the loaded classes to mock. | ||
| */ | ||
| @Canonical | ||
| private static class MockFeatures { | ||
| final Class<?> mockType | ||
| final List<Class<?>> interfaces | ||
| } | ||
|
|
||
| /** | ||
| * Class holding the class names to mock. | ||
| * Which will be converted into a {@link MockFeatures} during test. | ||
| */ | ||
| @Canonical | ||
| private static class MockSpec { | ||
| final String mockType | ||
| final List<String> interfaces | ||
| } | ||
| } |
55 changes: 55 additions & 0 deletions
55
spock-specs/src/test/groovy/org/spockframework/mock/runtime/ByteBuddyTestClassLoader.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| /* | ||
| * Copyright 2023 the original author or authors. | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * https://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package org.spockframework.mock.runtime; | ||
|
|
||
| import net.bytebuddy.ByteBuddy; | ||
| import net.bytebuddy.description.type.TypeDescription; | ||
| import net.bytebuddy.dynamic.loading.ClassLoadingStrategy; | ||
|
|
||
| import java.util.HashMap; | ||
| import java.util.Map; | ||
|
|
||
| final class ByteBuddyTestClassLoader extends ClassLoader { | ||
| private final Map<String, Class<?>> cache = new HashMap<>(); | ||
|
|
||
| private final ClassLoadingStrategy<ByteBuddyTestClassLoader> loadingStrategy = (loader, types) -> { | ||
| Map<TypeDescription, Class<?>> result = new HashMap<>(); | ||
| for (Map.Entry<TypeDescription, byte[]> entry : types.entrySet()) { | ||
| TypeDescription description = entry.getKey(); | ||
| byte[] bytes = entry.getValue(); | ||
| Class<?> clazz = defineClass(description.getName(), bytes, 0, bytes.length); | ||
| result.put(description, clazz); | ||
| } | ||
| return result; | ||
| }; | ||
|
|
||
| /** | ||
| * Defines an empty interface with the passed {@code node}. | ||
| * | ||
| * @param name the name of the interface | ||
| * @return the loaded {@code Class} | ||
| */ | ||
| synchronized Class<?> defineInterface(String name) { | ||
| //noinspection resource | ||
| return cache.computeIfAbsent(name, nameKey -> new ByteBuddy() | ||
| .makeInterface() | ||
| .name(nameKey) | ||
| .make() | ||
| .load(this, loadingStrategy) | ||
| .getLoaded()); | ||
| } | ||
| } |