Remove unnecessary synchronization

Mojang · Oct 12, 2018 · 0cc9840 · 0cc9840
1 parent a0318c3
commit 0cc9840
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 32 deletions.
diff --git a/src/main/java/com/mojang/datafixers/functions/PointFree.java b/src/main/java/com/mojang/datafixers/functions/PointFree.java
@@ -11,15 +11,19 @@
 import java.util.function.Function;
 
 public abstract class PointFree<T> {
-    private boolean initialized;
+    private volatile boolean initialized;
     @Nullable
     private Function<DynamicOps<?>, T> value;
 
     @SuppressWarnings("ConstantConditions")
-    public synchronized Function<DynamicOps<?>, T> evalCached() {
+    public Function<DynamicOps<?>, T> evalCached() {
         if (!initialized) {
-            initialized = true;
-            value = eval();
+            synchronized (this) {
+                if (!initialized) {
+                    value = eval();
+                    initialized = true;
+                }
+            }
         }
         return value;
     }

diff --git a/src/main/java/com/mojang/datafixers/schemas/Schema.java b/src/main/java/com/mojang/datafixers/schemas/Schema.java
@@ -13,7 +13,6 @@
 import com.mojang.datafixers.types.templates.TaggedChoice;
 import com.mojang.datafixers.types.templates.TypeTemplate;
 import it.unimi.dsi.fastutil.objects.Object2IntMap;
-import it.unimi.dsi.fastutil.objects.Object2IntMaps;
 import it.unimi.dsi.fastutil.objects.Object2IntOpenHashMap;
 
 import java.util.List;
@@ -23,8 +22,8 @@
 import java.util.function.Supplier;
 
 public class Schema {
-    protected final Object2IntMap<String> RECURSIVE_TYPES = Object2IntMaps.synchronize(new Object2IntOpenHashMap<>());
-    private final Map<String, Supplier<TypeTemplate>> TYPE_TEMPLATES = Maps.newConcurrentMap();
+    protected final Object2IntMap<String> RECURSIVE_TYPES = new Object2IntOpenHashMap<>();
+    private final Map<String, Supplier<TypeTemplate>> TYPE_TEMPLATES = Maps.newHashMap();
     private final Map<String, Type<?>> TYPES;
     private final int versionKey;
     private final String name;
@@ -40,30 +39,26 @@ public Schema(final int versionKey, final Schema parent) {
     }
 
     protected Map<String, Type<?>> buildTypes() {
-        final Map<String, Type<?>> types = Maps.newConcurrentMap();
+        final Map<String, Type<?>> types = Maps.newHashMap();
 
         final List<TypeTemplate> templates = Lists.newArrayList();
 
-        synchronized (RECURSIVE_TYPES) {
-            for (final Object2IntMap.Entry<String> entry : RECURSIVE_TYPES.object2IntEntrySet()) {
-                templates.add(DSL.check(entry.getKey(), entry.getIntValue(), getTemplate(entry.getKey())));
-            }
+        for (final Object2IntMap.Entry<String> entry : RECURSIVE_TYPES.object2IntEntrySet()) {
+            templates.add(DSL.check(entry.getKey(), entry.getIntValue(), getTemplate(entry.getKey())));
         }
 
         final TypeTemplate choice = templates.stream().reduce(DSL::or).get();
         final TypeFamily family = new RecursiveTypeFamily(name, choice);
 
-        synchronized (TYPE_TEMPLATES) {
-            for (final String name : TYPE_TEMPLATES.keySet()) {
-                final Type<?> type;
-                int recurseId = RECURSIVE_TYPES.getOrDefault(name, -1);
-                if (recurseId != -1) {
-                    type = family.apply(recurseId);
-                } else {
-                    type = getTemplate(name).apply(family).apply(-1);
-                }
-                types.put(name, type);
+        for (final String name : TYPE_TEMPLATES.keySet()) {
+            final Type<?> type;
+            int recurseId = RECURSIVE_TYPES.getOrDefault(name, -1);
+            if (recurseId != -1) {
+                type = family.apply(recurseId);
+            } else {
+                type = getTemplate(name).apply(family).apply(-1);
             }
+            types.put(name, type);
         }
         return types;
     }
@@ -147,10 +142,8 @@ public void register(final Map<String, Supplier<TypeTemplate>> map, final String
     public void registerType(final boolean recursive, final DSL.TypeReference type, final Supplier<TypeTemplate> template) {
         TYPE_TEMPLATES.put(type.typeName(), template);
         // TODO: calculate recursiveness instead of hardcoding
-        synchronized (RECURSIVE_TYPES) {
-            if (recursive && !RECURSIVE_TYPES.containsKey(type.typeName())) {
-                RECURSIVE_TYPES.put(type.typeName(), RECURSIVE_TYPES.size());
-            }
+        if (recursive && !RECURSIVE_TYPES.containsKey(type.typeName())) {
+            RECURSIVE_TYPES.put(type.typeName(), RECURSIVE_TYPES.size());
         }
     }
 

diff --git a/src/main/java/com/mojang/datafixers/types/Type.java b/src/main/java/com/mojang/datafixers/types/Type.java
@@ -32,8 +32,8 @@
 import java.util.concurrent.CompletableFuture;
 
 public abstract class Type<A> implements App<Type.Mu, A> {
-    private static final Map<Triple<Type<?>, TypeRewriteRule, PointFreeRule>, CompletableFuture<Optional<? extends RewriteResult<?, ?>>>> PENDING_REWRITE_CACHE = Maps.newConcurrentMap();
-    private static final Map<Triple<Type<?>, TypeRewriteRule, PointFreeRule>, Optional<? extends RewriteResult<?, ?>>> REWRITE_CACHE = Maps.newConcurrentMap();
+    private final Map<Pair<TypeRewriteRule, PointFreeRule>, CompletableFuture<Optional<? extends RewriteResult<A, ?>>>> PENDING_REWRITE_CACHE = Maps.newConcurrentMap();
+    private final Map<Pair<TypeRewriteRule, PointFreeRule>, Optional<RewriteResult<A, ?>>> REWRITE_CACHE = Maps.newConcurrentMap();
 
     public static class Mu implements K1 {}
 
@@ -158,16 +158,16 @@ public <T, B> T capWrite(final DynamicOps<T> ops, final Type<?> expectedType, fi
 
     @SuppressWarnings("unchecked")
     public Optional<RewriteResult<A, ?>> rewrite(final TypeRewriteRule rule, final PointFreeRule fRule) {
-        final Triple<Type<?>, TypeRewriteRule, PointFreeRule> key = Triple.of(this, rule, fRule);
+        final Pair<TypeRewriteRule, PointFreeRule> key = Pair.of(rule, fRule);
         // This code under contention would generate multiple rewrites, so we use CompletableFuture for pending rewrites.
         // We can not use computeIfAbsent because this is a recursive call that will block server startup
         // during the Bootstrap phrase that's trying to pre cache these rewrites.
-        Optional<? extends RewriteResult<?, ?>> rewrite = REWRITE_CACHE.get(key);
+        Optional<RewriteResult<A, ?>> rewrite = REWRITE_CACHE.get(key);
         //noinspection OptionalAssignedToNull
         if (rewrite != null) {
-            return (Optional<RewriteResult<A, ?>>) rewrite;
+            return rewrite;
         }
-        CompletableFuture<Optional<? extends RewriteResult<?, ?>>> pending;
+        CompletableFuture<Optional<? extends RewriteResult<A, ?>>> pending;
         boolean needsCreate;
         synchronized (PENDING_REWRITE_CACHE) {
             pending = PENDING_REWRITE_CACHE.get(key);