diff --git a/src/som/interpreter/objectstorage/ObjectTransitionSafepoint.java b/src/som/interpreter/objectstorage/ObjectTransitionSafepoint.java index 5817b0387..8e69ede7a 100644 --- a/src/som/interpreter/objectstorage/ObjectTransitionSafepoint.java +++ b/src/som/interpreter/objectstorage/ObjectTransitionSafepoint.java @@ -95,11 +95,13 @@ public void checkAndPerformSafepoint() { public void transitionObject(final SObject obj) { waitForSafepointStart(); - // Safepoint phase, used to update the object - // object is required to handle updates from multiple threads correctly - obj.updateLayoutToMatchClass(); - - phaser.finishSafepointAndAwaitCompletion(); + try { + // Safepoint phase, used to update the object + // object is required to handle updates from multiple threads correctly + obj.updateLayoutToMatchClass(); + } finally { + phaser.finishSafepointAndAwaitCompletion(); + } } /** @@ -113,11 +115,13 @@ public void writeUninitializedSlot(final SObject obj, final SlotDefinition slot, final Object value) { waitForSafepointStart(); - // Safepoint phase, used to update the object - // object is required to handle updates from multiple threads correctly - obj.writeUninitializedSlot(slot, value); - - phaser.finishSafepointAndAwaitCompletion(); + try { + // Safepoint phase, used to update the object + // object is required to handle updates from multiple threads correctly + obj.writeUninitializedSlot(slot, value); + } finally { + phaser.finishSafepointAndAwaitCompletion(); + } } /** @@ -131,22 +135,26 @@ public void writeAndGeneralizeSlot(final SObject obj, final SlotDefinition slot, final Object value) { waitForSafepointStart(); - // Safepoint phase, used to update the object - // object is required to handle updates from multiple threads correctly - obj.writeAndGeneralizeSlot(slot, value); - - phaser.finishSafepointAndAwaitCompletion(); + try { + // Safepoint phase, used to update the object + // object is required to handle updates from multiple threads correctly + obj.writeAndGeneralizeSlot(slot, value); + } finally { + phaser.finishSafepointAndAwaitCompletion(); + } } public void ensureSlotAllocatedToAvoidDeadlock(final SObject obj, final SlotDefinition slot) { waitForSafepointStart(); - // Safepoint phase, used to update the object - // object is required to handle updates from multiple threads correctly - obj.ensureSlotAllocatedToAvoidDeadlock(slot); - - phaser.finishSafepointAndAwaitCompletion(); + try { + // Safepoint phase, used to update the object + // object is required to handle updates from multiple threads correctly + obj.ensureSlotAllocatedToAvoidDeadlock(slot); + } finally { + phaser.finishSafepointAndAwaitCompletion(); + } } void renewAssumption() { diff --git a/src/som/interpreter/objectstorage/SafepointPhaser.java b/src/som/interpreter/objectstorage/SafepointPhaser.java index 989613cb2..87ca691e7 100644 --- a/src/som/interpreter/objectstorage/SafepointPhaser.java +++ b/src/som/interpreter/objectstorage/SafepointPhaser.java @@ -261,7 +261,7 @@ void arriveAtSafepointAndAwaitStart() { void finishSafepointAndAwaitCompletion() { int phase = arriveAndAwaitAdvance(); - assert (phase & 1) == 0 : "Expect phase to be evem on completion of safepoint, but was " + assert (phase & 1) == 0 : "Expect phase to be even on completion of safepoint, but was " + phase; } diff --git a/tests/java/som/interpreter/objectstorage/SafepointPhaserTest.java b/tests/java/som/interpreter/objectstorage/SafepointPhaserTest.java index 1de2d6734..f663a4ab0 100644 --- a/tests/java/som/interpreter/objectstorage/SafepointPhaserTest.java +++ b/tests/java/som/interpreter/objectstorage/SafepointPhaserTest.java @@ -64,58 +64,69 @@ public void testThreadsRegisterTriggerSafepointAndUnregister() throws Interrupte ParallelHelper.executeNTimesInParallel((final int id) -> { - ObjectTransitionSafepoint.INSTANCE.register(); - try { - barrier.await(); - } catch (InterruptedException | BrokenBarrierException e) { - throw new RuntimeException(e); - } + ObjectTransitionSafepoint.INSTANCE.register(); - ObjectTransitionSafepoint.INSTANCE.transitionObject( - new SMutableObject(instanceClass, factory, layout)); + try { + barrier.await(); + } catch (InterruptedException | BrokenBarrierException e) { + throw new RuntimeException(e); + } - ObjectTransitionSafepoint.INSTANCE.unregister(); + ObjectTransitionSafepoint.INSTANCE.transitionObject( + new SMutableObject(instanceClass, factory, layout)); + } finally { + ObjectTransitionSafepoint.INSTANCE.unregister(); + } return null; }); } + private void transitionObject() { + for (int i = 0; i < 1181; i += 1) { + ObjectTransitionSafepoint.INSTANCE.transitionObject( + new SMutableObject(instanceClass, factory, layout)); + } + } + @Test public void testSafepointStorm() throws InterruptedException { ObjectTransitionSafepoint.reset(); ParallelHelper.executeNTimesInParallel((final int id) -> { - ObjectTransitionSafepoint.INSTANCE.register(); + try { + ObjectTransitionSafepoint.INSTANCE.register(); - for (int i = 0; i < 500_000; i += 1) { - ObjectTransitionSafepoint.INSTANCE.transitionObject( - new SMutableObject(instanceClass, factory, layout)); + for (int i = 0; i < 1181; i += 1) { + transitionObject(); + } + } finally { + ObjectTransitionSafepoint.INSTANCE.unregister(); } - - ObjectTransitionSafepoint.INSTANCE.unregister(); return null; - }, 60); + }, 240); } @Test public void testSingleSafepointStorm() throws InterruptedException { ObjectTransitionSafepoint.reset(); ParallelHelper.executeNTimesInParallel((final int id) -> { + try { + ObjectTransitionSafepoint.INSTANCE.register(); - ObjectTransitionSafepoint.INSTANCE.register(); - - for (int i = 0; i < 100_000; i += 1) { - if (id == 0) { - ObjectTransitionSafepoint.INSTANCE.transitionObject( - new SMutableObject(instanceClass, factory, layout)); - } else { - ObjectTransitionSafepoint.INSTANCE.checkAndPerformSafepoint(); + for (int i = 0; i < 100_000; i += 1) { + if (id == 0) { + ObjectTransitionSafepoint.INSTANCE.transitionObject( + new SMutableObject(instanceClass, factory, layout)); + } else { + ObjectTransitionSafepoint.INSTANCE.checkAndPerformSafepoint(); + } } + } finally { + ObjectTransitionSafepoint.INSTANCE.unregister(); } - - ObjectTransitionSafepoint.INSTANCE.unregister(); return null; - }, 60); + }, 120); } @Test @@ -123,15 +134,17 @@ public void testSafepointRegisterStorm() throws InterruptedException { ObjectTransitionSafepoint.reset(); ParallelHelper.executeNTimesInParallel((final int id) -> { for (int i = 0; i < 100_000; i += 1) { - ObjectTransitionSafepoint.INSTANCE.register(); - - ObjectTransitionSafepoint.INSTANCE.transitionObject( - new SMutableObject(instanceClass, factory, layout)); + try { + ObjectTransitionSafepoint.INSTANCE.register(); - ObjectTransitionSafepoint.INSTANCE.unregister(); + ObjectTransitionSafepoint.INSTANCE.transitionObject( + new SMutableObject(instanceClass, factory, layout)); + } finally { + ObjectTransitionSafepoint.INSTANCE.unregister(); + } } return null; - }, 60); + }, 120); } @Test @@ -139,16 +152,18 @@ public void testSingleSafepointRegisterStorm() throws InterruptedException { ObjectTransitionSafepoint.reset(); ParallelHelper.executeNTimesInParallel((final int id) -> { for (int i = 0; i < 100_000; i += 1) { - ObjectTransitionSafepoint.INSTANCE.register(); - - if (id == 0) { - ObjectTransitionSafepoint.INSTANCE.transitionObject( - new SMutableObject(instanceClass, factory, layout)); - } else { - ObjectTransitionSafepoint.INSTANCE.checkAndPerformSafepoint(); + try { + ObjectTransitionSafepoint.INSTANCE.register(); + + if (id == 0) { + ObjectTransitionSafepoint.INSTANCE.transitionObject( + new SMutableObject(instanceClass, factory, layout)); + } else { + ObjectTransitionSafepoint.INSTANCE.checkAndPerformSafepoint(); + } + } finally { + ObjectTransitionSafepoint.INSTANCE.unregister(); } - - ObjectTransitionSafepoint.INSTANCE.unregister(); } return null; }, 60); @@ -159,8 +174,11 @@ public void testRegisterStorm() throws InterruptedException { ObjectTransitionSafepoint.reset(); ParallelHelper.executeNTimesInParallel((final int id) -> { for (int i = 0; i < 100_000; i += 1) { - ObjectTransitionSafepoint.INSTANCE.register(); - ObjectTransitionSafepoint.INSTANCE.unregister(); + try { + ObjectTransitionSafepoint.INSTANCE.register(); + } finally { + ObjectTransitionSafepoint.INSTANCE.unregister(); + } } return null; }, 60); diff --git a/tests/java/som/tests/ParallelHelper.java b/tests/java/som/tests/ParallelHelper.java index fd61d1a5f..41cf4f8ee 100644 --- a/tests/java/som/tests/ParallelHelper.java +++ b/tests/java/som/tests/ParallelHelper.java @@ -24,10 +24,9 @@ public static void executeNTimesInParallel(final IntFunction task, int numThreads = getNumberOfThreads(); ExecutorService threadPool = Executors.newFixedThreadPool(numThreads); + List exceptions = Collections.synchronizedList(new ArrayList()); try { - List exceptions = Collections.synchronizedList(new ArrayList()); - CountDownLatch threadsInitialized = new CountDownLatch(numThreads); CountDownLatch threadsDone = new CountDownLatch(numThreads); @@ -41,8 +40,9 @@ public static void executeNTimesInParallel(final IntFunction task, task.apply(id); } catch (Throwable t) { exceptions.add(t); + } finally { + threadsDone.countDown(); } - threadsDone.countDown(); }); } boolean allArrivedWithinTime = threadsDone.await(timeoutInSeconds, TimeUnit.SECONDS); @@ -53,7 +53,7 @@ public static void executeNTimesInParallel(final IntFunction task, } assertTrue("Failed parallel test with: " + exceptions, exceptions.isEmpty()); - assertTrue(allArrivedWithinTime); + assertTrue("Some threads timed out on threadsDone", allArrivedWithinTime); } finally { threadPool.shutdownNow(); }