diff --git a/SpecsUtils/src/pt/up/fe/specs/util/SpecsSystem.java b/SpecsUtils/src/pt/up/fe/specs/util/SpecsSystem.java index 43485977..34477cad 100644 --- a/SpecsUtils/src/pt/up/fe/specs/util/SpecsSystem.java +++ b/SpecsUtils/src/pt/up/fe/specs/util/SpecsSystem.java @@ -505,6 +505,7 @@ interface Returnable { } /** + * @param callGc whether to request garbage collection before measuring * @return the current amount of memory, in bytes */ public static long getUsedMemory(boolean callGc) { @@ -512,8 +513,6 @@ public static long getUsedMemory(boolean callGc) { System.gc(); } - System.gc(); - return Runtime.getRuntime().totalMemory() - Runtime.getRuntime().freeMemory(); } diff --git a/SpecsUtils/test/pt/up/fe/specs/util/SpecsSystemTest.java b/SpecsUtils/test/pt/up/fe/specs/util/SpecsSystemTest.java index 03658277..0563f179 100644 --- a/SpecsUtils/test/pt/up/fe/specs/util/SpecsSystemTest.java +++ b/SpecsUtils/test/pt/up/fe/specs/util/SpecsSystemTest.java @@ -16,11 +16,16 @@ import static org.assertj.core.api.Assertions.*; import java.io.File; +import java.io.IOException; +import java.nio.file.Path; import java.util.Arrays; import java.util.List; import java.util.concurrent.Callable; import java.util.concurrent.ThreadFactory; +import jdk.jfr.Recording; +import jdk.jfr.consumer.RecordingFile; + import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; @@ -443,6 +448,27 @@ void testIsAvailable() { @DisplayName("Memory and Performance") class MemoryPerformance { + @Test + @DisplayName("getUsedMemory should only request GC when asked") + void testGetUsedMemoryGcRequests(@TempDir Path tempDir) throws IOException { + assertThat(countGcRequests(false, tempDir.resolve("without-gc.jfr"))).isZero(); + assertThat(countGcRequests(true, tempDir.resolve("with-gc.jfr"))).isEqualTo(1); + } + + private long countGcRequests(boolean callGc, Path recordingPath) throws IOException { + try (var recording = new Recording()) { + recording.enable("jdk.SystemGC"); + recording.start(); + SpecsSystem.getUsedMemory(callGc); + recording.stop(); + recording.dump(recordingPath); + } + + return RecordingFile.readAllEvents(recordingPath).stream() + .filter(event -> event.getEventType().getName().equals("jdk.SystemGC")) + .count(); + } + @Test @DisplayName("getUsedMemory should return positive value") void testGetUsedMemory() { diff --git a/jOptions/src/org/suikasoft/jOptions/DataStore/ListDataStore.java b/jOptions/src/org/suikasoft/jOptions/DataStore/ListDataStore.java index 730a313f..72e2c677 100644 --- a/jOptions/src/org/suikasoft/jOptions/DataStore/ListDataStore.java +++ b/jOptions/src/org/suikasoft/jOptions/DataStore/ListDataStore.java @@ -20,6 +20,7 @@ import org.suikasoft.jOptions.storedefinition.StoreDefinitionIndexes; import java.util.*; +import java.util.concurrent.ConcurrentHashMap; /** * Implementation of DataStore that uses a List to store the data. @@ -32,7 +33,7 @@ */ public class ListDataStore implements DataStore { - private static final Map KEY_TO_INDEXES = new HashMap<>(); + private static final Map KEY_TO_INDEXES = new ConcurrentHashMap<>(); private final StoreDefinition keys; private final List values; @@ -334,12 +335,7 @@ private int toIndex(String key) { * @return the StoreDefinitionIndexes */ private StoreDefinitionIndexes getIndexes() { - StoreDefinitionIndexes indexes = KEY_TO_INDEXES.get(keys); - if (indexes == null) { - indexes = new StoreDefinitionIndexes(keys); - KEY_TO_INDEXES.put(keys, indexes); - } - return indexes; + return KEY_TO_INDEXES.computeIfAbsent(keys, StoreDefinitionIndexes::new); } /** diff --git a/jOptions/src/org/suikasoft/jOptions/Datakey/KeyFactory.java b/jOptions/src/org/suikasoft/jOptions/Datakey/KeyFactory.java index 85029980..553ae987 100644 --- a/jOptions/src/org/suikasoft/jOptions/Datakey/KeyFactory.java +++ b/jOptions/src/org/suikasoft/jOptions/Datakey/KeyFactory.java @@ -590,8 +590,7 @@ public static DataKey generic(String id, E exampleInstance) */ public static DataKey generic(String id, Supplier defaultSupplier) { DataKey datakey = new GenericKey<>(id, defaultSupplier.get()); - datakey.setDefault(defaultSupplier); - return datakey; + return datakey.setDefault(defaultSupplier); } /** diff --git a/jOptions/src/org/suikasoft/jOptions/storedefinition/AStoreDefinition.java b/jOptions/src/org/suikasoft/jOptions/storedefinition/AStoreDefinition.java index 6cf0cf8e..24808588 100644 --- a/jOptions/src/org/suikasoft/jOptions/storedefinition/AStoreDefinition.java +++ b/jOptions/src/org/suikasoft/jOptions/storedefinition/AStoreDefinition.java @@ -14,8 +14,9 @@ package org.suikasoft.jOptions.storedefinition; import java.util.ArrayList; -import java.util.HashMap; +import java.util.Collections; import java.util.HashSet; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Set; @@ -31,7 +32,7 @@ public abstract class AStoreDefinition implements StoreDefinition { private final String appName; private final List sections; private final DataStore defaultData; - private final Map> keyMap = new HashMap<>(); + private final Map> keyMap; /** * Creates a new store definition with the given name and options. @@ -56,13 +57,17 @@ protected AStoreDefinition(String appName, List sections, DataStor this.appName = appName; this.sections = new ArrayList<>(sections); this.defaultData = defaultData; + Map> keysByName = new LinkedHashMap<>(); + for (StoreSection section : this.sections) { + for (DataKey key : section.getKeys()) { + keysByName.put(key.getName(), key); + } + } + this.keyMap = Collections.unmodifiableMap(keysByName); } @Override public Map> getKeyMap() { - if (keyMap.isEmpty()) { - keyMap.putAll(StoreDefinition.super.getKeyMap()); - } return keyMap; } diff --git a/jOptions/test/org/suikasoft/jOptions/Datakey/KeyFactoryTest.java b/jOptions/test/org/suikasoft/jOptions/Datakey/KeyFactoryTest.java index 5d1d2af5..9527b90a 100644 --- a/jOptions/test/org/suikasoft/jOptions/Datakey/KeyFactoryTest.java +++ b/jOptions/test/org/suikasoft/jOptions/Datakey/KeyFactoryTest.java @@ -5,11 +5,14 @@ import java.io.File; import java.math.BigInteger; import java.util.List; +import java.util.concurrent.atomic.AtomicInteger; import org.junit.jupiter.api.DisplayName; import pt.up.fe.specs.util.utilities.StringList; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; +import org.suikasoft.jOptions.Interfaces.DataStore; +import org.suikasoft.jOptions.storedefinition.StoreDefinition; /** * Comprehensive test suite for KeyFactory static factory methods. @@ -218,6 +221,64 @@ void testStringListFactoryWithDefault_CreatesStringListDataKey() { assertThat(key.getName()).isEqualTo("default.stringlist"); assertThat(key.getValueClass()).isEqualTo(StringList.class); } + + @Test + @DisplayName("list defaults are present, mutable, and independent across stores") + @SuppressWarnings({ "rawtypes", "unchecked" }) + void testListFactory_DefaultsAreIndependentAcrossStores_AndSetterValidatesElements() { + DataKey> key = KeyFactory.list("typed.list", String.class); + StoreDefinition definition = StoreDefinition.newInstance("Typed Lists", key); + DataStore simpleFirst = DataStore.newInstance(definition); + DataStore simpleSecond = DataStore.newInstance(definition); + DataStore closedFirst = DataStore.newInstance(definition, true); + DataStore closedSecond = DataStore.newInstance(definition, true); + + List simpleFirstValue = simpleFirst.get(key); + List simpleSecondValue = simpleSecond.get(key); + List closedFirstValue = closedFirst.get(key); + List closedSecondValue = closedSecond.get(key); + List> defaults = List.of(simpleFirstValue, simpleSecondValue, + closedFirstValue, closedSecondValue); + + assertThat(key.hasDefaultValue()).isTrue(); + assertThat(key.getDefault()).hasValueSatisfying(value -> assertThat(value).isEmpty()); + assertThat(defaults).allSatisfy(value -> assertThat(value).isEmpty()); + + for (int i = 0; i < defaults.size(); i++) { + for (int j = i + 1; j < defaults.size(); j++) { + assertThat(defaults.get(i)).isNotSameAs(defaults.get(j)); + } + } + + simpleFirstValue.add("mutable"); + assertThat(simpleSecondValue).isEmpty(); + assertThat(closedFirstValue).isEmpty(); + assertThat(closedSecondValue).isEmpty(); + + assertThatThrownBy(() -> simpleFirst.set(key, (List) List.of(1))) + .isInstanceOf(ClassCastException.class); + } + } + + @Nested + @DisplayName("Generic Key Factory") + class GenericFactoryTests { + + @Test + @DisplayName("generic factory retains and invokes its default supplier") + void testGenericFactory_RetainsDefaultSupplier() { + AtomicInteger supplierCalls = new AtomicInteger(); + + DataKey key = KeyFactory.generic("supplier.default", () -> { + supplierCalls.incrementAndGet(); + return "default"; + }); + + assertThat(supplierCalls).hasValue(1); + assertThat(key.hasDefaultValue()).isTrue(); + assertThat(key.getDefault()).hasValue("default"); + assertThat(supplierCalls).hasValue(2); + } } @Nested diff --git a/jOptions/test/org/suikasoft/jOptions/storedefinition/AStoreDefinitionConcurrencyTest.java b/jOptions/test/org/suikasoft/jOptions/storedefinition/AStoreDefinitionConcurrencyTest.java new file mode 100644 index 00000000..5ed2a5de --- /dev/null +++ b/jOptions/test/org/suikasoft/jOptions/storedefinition/AStoreDefinitionConcurrencyTest.java @@ -0,0 +1,84 @@ +package org.suikasoft.jOptions.storedefinition; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.concurrent.CyclicBarrier; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.stream.IntStream; + +import org.junit.jupiter.api.Test; +import org.suikasoft.jOptions.DataStore.ListDataStore; +import org.suikasoft.jOptions.Datakey.DataKey; +import org.suikasoft.jOptions.Datakey.KeyFactory; + +class AStoreDefinitionConcurrencyTest { + + private static final int THREADS = 32; + private static final int SHARED_KEYS = 2_048; + private static final int STORE_KEYS = 8; + private static final int ROUNDS = 8; + + @Test + void concurrentFirstReadersAndStoreCreationSeeCompleteDefinitions() throws Exception { + ExecutorService executor = Executors.newFixedThreadPool(THREADS); + + try { + for (int round = 0; round < ROUNDS; round++) { + final int currentRound = round; + List> sharedKeys = keys("shared-" + currentRound + "-", SHARED_KEYS); + TestStoreDefinition shared = new TestStoreDefinition("shared-" + currentRound, sharedKeys); + List independent = IntStream.range(0, THREADS) + .mapToObj(thread -> new TestStoreDefinition("independent-" + currentRound + "-" + thread, + keys("independent-" + currentRound + "-" + thread + "-", STORE_KEYS))) + .toList(); + CyclicBarrier gate = new CyclicBarrier(THREADS + 1); + List> futures = new ArrayList<>(THREADS); + + for (int thread = 0; thread < THREADS; thread++) { + final int threadIndex = thread; + futures.add(executor.submit(() -> { + gate.await(); + + Map> map = shared.getKeyMap(); + TestStoreDefinition definition = independent.get(threadIndex); + DataKey firstKey = definition.getKeys().get(0); + DataKey secondKey = definition.getKeys().get(1); + ListDataStore eager = new ListDataStore(definition); + eager.setRaw(firstKey.getName(), "eager"); + eager.get(firstKey.getName()); + eager.setRaw(secondKey.getName(), "second"); + eager.get(secondKey.getName()); + + return map.size(); + })); + } + + gate.await(); + for (Future future : futures) { + assertThat(future.get()).isEqualTo(SHARED_KEYS); + } + } + } finally { + executor.shutdownNow(); + } + } + + private static List> keys(String prefix, int count) { + List> keys = new ArrayList<>(count); + for (int index = 0; index < count; index++) { + keys.add(KeyFactory.string(prefix + index)); + } + return keys; + } + + private static final class TestStoreDefinition extends AStoreDefinition { + private TestStoreDefinition(String name, List> keys) { + super(name, keys); + } + } +} diff --git a/jOptions/test/org/suikasoft/jOptions/storedefinition/AStoreDefinitionTest.java b/jOptions/test/org/suikasoft/jOptions/storedefinition/AStoreDefinitionTest.java index b26a28ff..871e8765 100644 --- a/jOptions/test/org/suikasoft/jOptions/storedefinition/AStoreDefinitionTest.java +++ b/jOptions/test/org/suikasoft/jOptions/storedefinition/AStoreDefinitionTest.java @@ -118,6 +118,26 @@ void shouldHandleNullSectionsList() { assertThatThrownBy(() -> new TestStoreDefinition("NullSections", null, null)) .isInstanceOf(NullPointerException.class); } + + @Test + void constructionDoesNotCallSubclassKeyAccessors() { + class DefinitionWithInitializedKeys extends AStoreDefinition { + private final List> initializedKeys; + + DefinitionWithInitializedKeys(List> keys) { + super("subclass", keys); + initializedKeys = keys; + } + + @Override + public List> getKeys() { + return List.copyOf(initializedKeys); + } + } + + var definition = new DefinitionWithInitializedKeys(testKeys); + assertThat(definition.getKeyMap().values()).containsExactlyElementsOf(testKeys); + } } @Nested @@ -178,6 +198,22 @@ void shouldCacheKeyMap() { assertThat(keyMap1).isSameAs(keyMap2); } + @Test + void publishedKeyMapCannotBeMutated() { + assertThatThrownBy(() -> storeDefinition.getKeyMap().remove(testStringKey.getName())) + .isInstanceOf(UnsupportedOperationException.class); + assertThat(storeDefinition.getKeyMap()).containsKey(testStringKey.getName()); + } + + @Test + void keyMapPreservesOrderAcrossSections() { + var first = StoreSection.newInstance(List.of(testBoolKey, testStringKey)); + var second = StoreSection.newInstance(List.of(testIntKey)); + var definition = new TestStoreDefinition("ordered", List.of(first, second), null); + + assertThat(definition.getKeyMap().keySet()).containsExactly("testBool", "testString", "testInt"); + } + @Test @DisplayName("Should handle empty key list") void shouldHandleEmptyKeyList() {