From 5d6566512c9b434070bbe231ac6c7d09c68569b0 Mon Sep 17 00:00:00 2001 From: embeddedt <42941056+embeddedt@users.noreply.github.com> Date: Tue, 20 Jun 2023 10:31:49 -0400 Subject: [PATCH] Rewrite CanonizingStringMap to simply use a normal HashMap and intern keys There are no memory savings from using the fastutil maps, and they may be harming performance based on the Project MMO issues Probably also the solution to #134 --- .../modernfix/util/CanonizingStringMap.java | 157 +++--------------- 1 file changed, 23 insertions(+), 134 deletions(-) diff --git a/common/src/main/java/org/embeddedt/modernfix/util/CanonizingStringMap.java b/common/src/main/java/org/embeddedt/modernfix/util/CanonizingStringMap.java index 1202500b..352492b1 100644 --- a/common/src/main/java/org/embeddedt/modernfix/util/CanonizingStringMap.java +++ b/common/src/main/java/org/embeddedt/modernfix/util/CanonizingStringMap.java @@ -1,157 +1,46 @@ package org.embeddedt.modernfix.util; +import com.google.common.base.Function; import com.google.common.collect.Interner; import com.google.common.collect.Interners; -import it.unimi.dsi.fastutil.objects.*; -import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; +import com.google.common.collect.Maps; -import java.util.*; -import java.util.function.Function; +import java.util.HashMap; +import java.util.Map; /** - * Replacement backing map for CompoundTags. Uses an array map for tags with 4 or less entries, - * and a hash map for larger tags. + * Replacement backing map for CompoundTags that interns keys. */ -public class CanonizingStringMap implements Map { - private Object2ObjectMap backingMap; - - private static final int GROWTH_THRESHOLD = 4; +public class CanonizingStringMap extends HashMap { private static final Interner KEY_INTERNER = Interners.newStrongInterner(); + private static String intern(String key) { + return key != null ? KEY_INTERNER.intern(key) : null; + } + public CanonizingStringMap() { - this(new Object2ObjectArrayMap<>()); - } - - protected CanonizingStringMap(Object2ObjectMap newMap) { - this.backingMap = newMap; + super(); } @Override - public int size() { - return backingMap.size(); + public T put(String key, T value) { + return super.put(intern(key), value); } @Override - public boolean isEmpty() { - return backingMap.isEmpty(); + public void putAll(Map m) { + HashMap tmp = new HashMap<>(); + m.forEach((k, v) -> tmp.put(intern(k), v)); + super.putAll(tmp); } - @Override - public boolean containsKey(Object o) { - return backingMap.containsKey(o); + private void putAllWithoutInterning(Map m) { + super.putAll(m); } - @Override - public boolean containsValue(Object o) { - return backingMap.containsValue(o); - } - - @Override - public T get(Object o) { - return backingMap.get(o); - } - - @Nullable - @Override - public T put(String s, T t) { - if(backingMap.size() >= GROWTH_THRESHOLD && !(backingMap instanceof Object2ObjectOpenHashMap) && !backingMap.containsKey(s)) { - // map will grow to GROWTH_THRESHOLD + 1 entries, change to hashmap - backingMap = new Object2ObjectOpenHashMap<>(backingMap); - } - s = KEY_INTERNER.intern(s); - return backingMap.put(s, t); - } - - @Override - public T remove(Object o) { - T value = backingMap.remove(o); - // need to shrink to be consistent with new maps - if(backingMap.size() <= GROWTH_THRESHOLD && backingMap instanceof Object2ObjectOpenHashMap) { - backingMap = new Object2ObjectArrayMap<>(backingMap); - } - return value; - } - - @Override - public void putAll(@NotNull Map map) { - if(map.size() == 0) - return; - // grow early if we know there are enough non-overlapping keys - if((map.size() - backingMap.size()) > GROWTH_THRESHOLD && !(backingMap instanceof Object2ObjectOpenHashMap)) { - backingMap = new Object2ObjectOpenHashMap<>(backingMap); - } - map.forEach((String key, T val) -> { - key = KEY_INTERNER.intern(key); - backingMap.put(key, val); - }); - // if it's still an array, and now too big, grow it - if(backingMap.size() > GROWTH_THRESHOLD && !(backingMap instanceof Object2ObjectOpenHashMap)) { - backingMap = new Object2ObjectOpenHashMap<>(backingMap); - } - } - - @Override - public void clear() { - if(!(this.backingMap instanceof Object2ObjectArrayMap)) - this.backingMap = new Object2ObjectArrayMap<>(); - else - this.backingMap.clear(); - } - - @NotNull - @Override - public Set keySet() { - // has to be modifiable because mods (cough, Tinkers) use it to clear the tag - return this.backingMap.keySet(); - } - - @NotNull - @Override - public Collection values() { - return Collections.unmodifiableCollection(this.backingMap.values()); - } - - @NotNull - @Override - public Set> entrySet() { - return Collections.unmodifiableSet(this.backingMap.entrySet()); - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - CanonizingStringMap that = (CanonizingStringMap)o; - if(that.backingMap.size() != backingMap.size()) - return false; - return backingMap.object2ObjectEntrySet().containsAll(that.backingMap.object2ObjectEntrySet()); - } - - /** - * We deliberately use a hashcode that will be consistent regardless of underlying map type. - */ - @Override - public int hashCode() { - final ObjectIterator> i = Object2ObjectMaps.fastIterator(backingMap); - int h = 0, n = backingMap.size(); - while (n-- != 0) - h += i.next().hashCode(); - return h; - } - - public static CanonizingStringMap deepCopy(CanonizingStringMap inputMap, Function deepCopier) { - Objects.requireNonNull(deepCopier); - Object2ObjectMap copiedBackingMap; - int size = inputMap.backingMap.size(); - if(size > GROWTH_THRESHOLD) { - copiedBackingMap = new Object2ObjectOpenHashMap<>(size); - } else - copiedBackingMap = new Object2ObjectArrayMap<>(size); - inputMap.backingMap.object2ObjectEntrySet().forEach(entry -> { - if(entry.getKey() != null && entry.getValue() != null) - copiedBackingMap.put(entry.getKey(), deepCopier.apply(entry.getValue())); - }); - return new CanonizingStringMap<>(copiedBackingMap); + public static CanonizingStringMap deepCopy(CanonizingStringMap incomingMap, Function deepCopier) { + CanonizingStringMap newMap = new CanonizingStringMap<>(); + newMap.putAllWithoutInterning(Maps.transformValues(incomingMap, deepCopier)); + return newMap; } }