Prefer performance over thread-safety for hashCode computation. Use a…

… boolean rather than a ThreadLocal
dserfe · Aug 23, 2023 · 9607209 · 9607209
1 parent 74a88a7
commit 9607209
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 22 deletions.
diff --git a/src/main/java/com/hubspot/jinjava/objects/collections/PyList.java b/src/main/java/com/hubspot/jinjava/objects/collections/PyList.java
@@ -7,12 +7,9 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.Objects;
-import java.util.concurrent.Semaphore;
 
 public class PyList extends ForwardingList<Object> implements PyWrapper {
-  private final ThreadLocal<Semaphore> semaphore = ThreadLocal.withInitial(
-    () -> new Semaphore(1)
-  );
+  private boolean computingHashCode = false;
   private final List<Object> list;
 
   public PyList(List<Object> list) {
@@ -104,16 +101,20 @@ IndexOutOfRangeException createOutOfRangeException(int index) {
     );
   }
 
+  /**
+   * This is not thread-safe
+   * @return hashCode, preventing recursion
+   */
   @Override
   public int hashCode() {
-    if (semaphore.get().tryAcquire()) {
-      try {
-        return super.hashCode();
-      } finally {
-        semaphore.get().release();
-      }
-    } else {
+    if (computingHashCode) {
       return Objects.hashCode(null);
     }
+    try {
+      computingHashCode = true;
+      return super.hashCode();
+    } finally {
+      computingHashCode = false;
+    }
   }
 }
diff --git a/src/main/java/com/hubspot/jinjava/objects/collections/PyMap.java b/src/main/java/com/hubspot/jinjava/objects/collections/PyMap.java
@@ -5,12 +5,9 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
-import java.util.concurrent.Semaphore;
 
 public class PyMap extends ForwardingMap<String, Object> implements PyWrapper {
-  private final ThreadLocal<Semaphore> semaphore = ThreadLocal.withInitial(
-    () -> new Semaphore(1)
-  );
+  private boolean computingHashCode = false;
 
   private final Map<String, Object> map;
 
@@ -65,16 +62,20 @@ public void putAll(Map<? extends String, ? extends Object> m) {
     super.putAll(m);
   }
 
+  /**
+   * This is not thread-safe
+   * @return hashCode, preventing recursion
+   */
   @Override
   public int hashCode() {
-    if (semaphore.get().tryAcquire()) {
-      try {
-        return super.hashCode();
-      } finally {
-        semaphore.get().release();
-      }
-    } else {
+    if (computingHashCode) {
       return Objects.hashCode(null);
     }
+    try {
+      computingHashCode = true;
+      return super.hashCode();
+    } finally {
+      computingHashCode = false;
+    }
   }
 }