Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 3 additions & 7 deletions jOptions/src/org/suikasoft/jOptions/DataStore/ListDataStore.java
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -32,7 +33,7 @@
*/
public class ListDataStore implements DataStore {

private static final Map<StoreDefinition, StoreDefinitionIndexes> KEY_TO_INDEXES = new HashMap<>();
private static final Map<StoreDefinition, StoreDefinitionIndexes> KEY_TO_INDEXES = new ConcurrentHashMap<>();

private final StoreDefinition keys;
private final List<Object> values;
Expand Down Expand Up @@ -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);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public abstract class AStoreDefinition implements StoreDefinition {
private final String appName;
private final List<StoreSection> sections;
private final DataStore defaultData;
private final Map<String, DataKey<?>> keyMap = new HashMap<>();
private final Map<String, DataKey<?>> keyMap;

/**
* Creates a new store definition with the given name and options.
Expand All @@ -56,13 +56,11 @@ protected AStoreDefinition(String appName, List<StoreSection> sections, DataStor
this.appName = appName;
this.sections = new ArrayList<>(sections);
this.defaultData = defaultData;
this.keyMap = new HashMap<>(StoreDefinition.super.getKeyMap());

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Build the key map without dispatching to subclasses

StoreDefinition.super.getKeyMap() calls getKeys() virtually. If a downstream AStoreDefinition subclass overrides getKeys() and reads fields initialized after super(...), construction now sees uninitialized state and can throw or permanently cache an incomplete map; previously the lazy lookup occurred only after construction. Populate the map directly from the initialized sections or through a non-overridable base method.

Useful? React with 👍 / 👎.

}

@Override
public Map<String, DataKey<?>> getKeyMap() {
if (keyMap.isEmpty()) {
keyMap.putAll(StoreDefinition.super.getKeyMap());
}
return keyMap;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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<DataKey<?>> sharedKeys = keys("shared-" + currentRound + "-", SHARED_KEYS);
TestStoreDefinition shared = new TestStoreDefinition("shared-" + currentRound, sharedKeys);
List<TestStoreDefinition> 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<Future<Integer>> futures = new ArrayList<>(THREADS);

for (int thread = 0; thread < THREADS; thread++) {
final int threadIndex = thread;
futures.add(executor.submit(() -> {
gate.await();

Map<String, DataKey<?>> 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<Integer> future : futures) {
assertThat(future.get()).isEqualTo(SHARED_KEYS);
}
}
} finally {
executor.shutdownNow();
}
}

private static List<DataKey<?>> keys(String prefix, int count) {
List<DataKey<?>> 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<DataKey<?>> keys) {
super(name, keys);
}
}
}
Loading