From a8aa074328cceeb84be850dbd97b61f3b81de732 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Wed, 27 May 2020 09:31:03 +0200 Subject: [PATCH] DATACMNS-1735 - Polishing. Use mutated object as guard for synchronization. Copy discovered callbacks to cached callbacks. Reduce concurrency in unit test to reduce test load. Guard synchronization with timeouts. Original pull request: #446. --- .../callback/EntityCallbackDiscoverer.java | 5 ++-- .../EntityCallbackDiscovererUnitTests.java | 27 ++++++++++++------- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/src/main/java/org/springframework/data/mapping/callback/EntityCallbackDiscoverer.java b/src/main/java/org/springframework/data/mapping/callback/EntityCallbackDiscoverer.java index 6e6472215..56cf9087b 100644 --- a/src/main/java/org/springframework/data/mapping/callback/EntityCallbackDiscoverer.java +++ b/src/main/java/org/springframework/data/mapping/callback/EntityCallbackDiscoverer.java @@ -384,10 +384,11 @@ class EntityCallbackDiscoverer { if (this.entityCallbackBeans.isEmpty()) { if (cachedEntityCallbacks.size() != entityCallbacks.size()) { - List> entityCallbacks = new ArrayList<>(this.entityCallbacks.size()); + + List> entityCallbacks = new ArrayList<>(this.entityCallbacks); AnnotationAwareOrderComparator.sort(entityCallbacks); - synchronized(this) { + synchronized (cachedEntityCallbacks) { cachedEntityCallbacks.clear(); cachedEntityCallbacks.addAll(entityCallbacks); } diff --git a/src/test/java/org/springframework/data/mapping/callback/EntityCallbackDiscovererUnitTests.java b/src/test/java/org/springframework/data/mapping/callback/EntityCallbackDiscovererUnitTests.java index 5d9c24bca..3793283e2 100644 --- a/src/test/java/org/springframework/data/mapping/callback/EntityCallbackDiscovererUnitTests.java +++ b/src/test/java/org/springframework/data/mapping/callback/EntityCallbackDiscovererUnitTests.java @@ -21,6 +21,9 @@ import java.util.Collection; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.LinkedBlockingDeque; +import java.util.concurrent.ThreadPoolExecutor; +import java.util.concurrent.TimeUnit; import org.junit.jupiter.api.Test; import org.springframework.context.annotation.AnnotationConfigApplicationContext; @@ -35,8 +38,11 @@ import org.springframework.data.mapping.Person; import org.springframework.data.mapping.PersonDocument; /** + * Unit tests for {@link EntityCallbackDiscoverer}. + * * @author Christoph Strobl * @author Myeonghyeon Lee + * @author Mark Paluch */ class EntityCallbackDiscovererUnitTests { @@ -59,30 +65,31 @@ class EntityCallbackDiscovererUnitTests { AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyConfig.class); EntityCallbackDiscoverer discoverer = new EntityCallbackDiscoverer(ctx); - - int concurrencyCount = 4000; - CountDownLatch startLatch = new CountDownLatch(concurrencyCount); - CountDownLatch doneLatch = new CountDownLatch(concurrencyCount); + int poolSize = Runtime.getRuntime().availableProcessors(); + ThreadPoolExecutor executor = new ThreadPoolExecutor(poolSize, poolSize, 20, TimeUnit.SECONDS, + new LinkedBlockingDeque<>()); + CountDownLatch startLatch = new CountDownLatch(poolSize); + CountDownLatch doneLatch = new CountDownLatch(poolSize); List exceptions = new CopyOnWriteArrayList<>(); - for (int i = 0; i < concurrencyCount; i++) { - Thread thread = new Thread(() -> { + for (int i = 0; i < poolSize; i++) { + executor.submit(() -> { try { startLatch.countDown(); - startLatch.await(); + startLatch.await(5, TimeUnit.SECONDS); discoverer.getEntityCallbacks(PersonDocument.class, - ResolvableType.forType(BeforeSaveCallback.class)); + ResolvableType.forType(BeforeSaveCallback.class)); } catch (Exception ex) { exceptions.add(ex); } finally { doneLatch.countDown(); } }); - thread.start(); } - doneLatch.await(); + doneLatch.await(10, TimeUnit.SECONDS); + executor.shutdownNow(); assertThat(exceptions).isEmpty(); }