I've built a collection of atomic maps, sets and lists classes using AtomicReference. I've ran into a problem where when AtomicReference#compareAndSet is called ~20 times a second there is severe lag. (AtomicSet doesn't cause any lag, but that's only because it doesn't call compareAndSet if it already contains the object)
If I comment out the method and simply add synchronized to the method, the lag goes away, but is that a valid substitute and have I over-engineered for something I didn't need to?
I've tried a bunch of different combinations of method layout where I only get the current map and create a new modified map once, then just repeatedly call compareAndSet until it returns true, still lag. I've also tried just calling AtomicReference#set once, which also lags.
Summary: My AtomicReference#compareAndSet calls bog down the main thread, and I'm looking to find out if I've completely misused AtomicReference, should be using synchronized or a fix for my code, if there is one.
For context, I'm running the following code inside a Minecraft Sponge Mixin injection. All it does is inject my java code at the bottom of the defined method (which appears to run at the Minecraft tick speed, 20 times a second):
package net.netcoding.mod.mixins;
import net.minecraft.tileentity.TileEntityHopper;
import net.minecraft.tileentity.TileEntityLockable;
import net.minecraft.util.math.BlockPos;
import net.netcoding.mod.util.Cache;
import org.spongepowered.asm.mixin.Mixin;
import org.spongepowered.asm.mixin.Shadow;
import org.spongepowered.asm.mixin.injection.At;
import org.spongepowered.asm.mixin.injection.Inject;
import org.spongepowered.asm.mixin.injection.callback.CallbackInfo;
@Mixin(TileEntityHopper.class)
public abstract class MixinTileEntityHopper extends TileEntityLockable {
@Shadow public abstract double getXPos();
@Shadow public abstract double getZPos();
@Shadow public abstract double getYPos();
@Inject(
method = "update",
at = @At(
value = "TAIL"
)
)
private void mod_update(CallbackInfo ci) {
BlockPos position = new BlockPos(this.getXPos(), this.getYPos(), this.getZPos());
Cache.STORED_HOPPERS.put(position, true); // this line calls the AtomicMap#put
}
}
Here's the AtomicMap class in question (please take a look at the put and put2 methods):
package net.netcoding.core.api.util.concurrent.atomic;
import java.util.*;
import java.util.concurrent.atomic.AtomicReference;
import java.util.stream.Stream;
public abstract class AtomicMap<K, V, M extends AbstractMap<K, V>> extends AbstractMap<K, V> implements Iterable<Map.Entry<K, V>>, Map<K, V> {
protected final AtomicReference<M> ref;
/**
* Create a new concurrent map.
*/
protected AtomicMap(M type) {
this.ref = new AtomicReference<>(type);
}
@Override
public final void clear() {
this.ref.get().clear();
}
@Override
public final boolean containsKey(Object key) {
return this.ref.get().containsKey(key);
}
@Override
public final boolean containsValue(Object value) {
return this.ref.get().containsValue(value);
}
@Override
public final Set<Entry<K, V>> entrySet() {
return this.ref.get().entrySet();
}
@Override
public final V get(Object key) {
return this.ref.get().get(key);
}
@Override
public final V getOrDefault(Object key, V defaultValue) {
M current = this.ref.get();
return current.getOrDefault(key, defaultValue);
}
@Override
public final boolean isEmpty() {
return this.ref.get().isEmpty();
}
@Override
public Iterator<Entry<K, V>> iterator() {
return this.entrySet().iterator();
}
@Override
public final Set<K> keySet() {
return this.ref.get().keySet();
}
@SuppressWarnings("unchecked")
private M newMap(M current) {
try {
Map<K, V> map = current.getClass().newInstance();
map.putAll(current);
return (M)map;
} catch (Exception ex) {
throw new RuntimeException("Unable to create new map instance of " + current.getClass().getSimpleName() + "!");
}
}
public final Stream<Entry<K, V>> parallelStream() {
return this.entrySet().parallelStream();
}
@Override
public final V put(K key, V value) {
while (true) {
M current = this.ref.get();
M modified = this.newMap(current);
V old = modified.put(key, value);
// Causes severe lag if called ~20 times a second
if (this.ref.compareAndSet(current, modified))
return old;
}
}
// Causes no lag, but answers in questions about AtomicReference and synchronized
// all say synchronized can deadlock a thread, and hang until it's complete
// which is actually the exact opposite of what I am experiencing
// and I'm not sure this is even a correct way to use AtomicReference
public synchronized final V put2(K key, V value) {
return this.ref.get().put(key, value);
}
@Override
public synchronized final void putAll(Map<? extends K, ? extends V> map) {
this.ref.get().putAll(map);
/*while (true) {
M current = this.ref.get();
M modified = this.newMap(current);
modified.putAll(map);
if (this.ref.compareAndSet(current, modified))
return;
}*/
}
@Override
public synchronized final V putIfAbsent(K key, V value) {
return this.ref.get().putIfAbsent(key, value);
/*while (true) {
M current = this.ref.get();
M modified = this.newMap(current);
if (!modified.containsKey(key) || modified.get(key) == null) {
V old = modified.put(key, value);
if (this.ref.compareAndSet(current, modified))
return old;
} else
return null;
}*/
}
@Override
public final V remove(Object key) {
while (true) {
M current = this.ref.get();
if (!current.containsKey(key))
return null;
M modified = this.newMap(current);
V old = modified.remove(key);
if (this.ref.compareAndSet(current, modified))
return old;
}
}
@Override
public final boolean remove(Object key, Object value) {
while (true) {
M current = this.ref.get();
if (!current.containsKey(key))
return false;
M modified = this.newMap(current);
V currentValue = modified.get(key);
if (Objects.equals(currentValue, value)) {
modified.remove(key);
if (this.ref.compareAndSet(current, modified))
return true;
} else
return false;
}
}
@Override
public final int size() {
return this.ref.get().size();
}
public final Stream<Entry<K, V>> stream() {
return this.entrySet().stream();
}
@Override
public final Collection<V> values() {
return this.ref.get().values();
}
}
ConcurrentMap (implementation of AtomicMap)
package net.netcoding.core.api.util.concurrent;
import net.netcoding.core.api.util.concurrent.atomic.AtomicMap;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.atomic.AtomicReference;
public class ConcurrentMap<K, V> extends AtomicMap<K, V, HashMap<K, V>> {
/**
* Create a new concurrent map.
*/
public ConcurrentMap() {
super(new HashMap<>());
}
/**
* Create a new concurrent map and fill it with the given map.
*/
public ConcurrentMap(Map<? extends K, ? extends V> map) {
super(new HashMap<>(map));
}
}
