remove gil from HashingStorageLibrary exports, since this is an internal library

Author: timfel

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/common/DynamicObjectStorage.java

@@ -48,10 +48,8 @@
 import com.oracle.graal.python.builtins.objects.common.HashingStorageLibrary.HashingStorageIterable;
 import com.oracle.graal.python.builtins.objects.function.PArguments.ThreadState;
 import com.oracle.graal.python.builtins.objects.object.PythonObjectLibrary;
-import com.oracle.graal.python.runtime.GilNode;
 import com.oracle.truffle.api.dsl.Cached;
 import com.oracle.truffle.api.dsl.Cached.Exclusive;
-import com.oracle.truffle.api.dsl.Cached.Shared;
 import com.oracle.truffle.api.dsl.CachedLanguage;
 import com.oracle.truffle.api.library.CachedLibrary;
 import com.oracle.truffle.api.library.ExportLibrary;
@@ -70,53 +68,38 @@ public int length() {
     @ExportMessage(limit = "1")
     public Object getItemWithState(Object key, ThreadState state,
                     @CachedLibrary("key") PythonObjectLibrary lib,
-                    @Exclusive @Cached("createBinaryProfile()") ConditionProfile gotState, @Shared("gil") @Cached GilNode gil) {
-        boolean mustRelease = gil.acquire();
-        try {
-            // must call __hash__ for potential side-effect
-            getHashWithState(key, lib, state, gotState);
-            return null;
-        } finally {
-            gil.release(mustRelease);
-        }
+                    @Exclusive @Cached("createBinaryProfile()") ConditionProfile gotState) {
+        // must call __hash__ for potential side-effect
+        getHashWithState(key, lib, state, gotState);
+        return null;
     }
 
     @ExportMessage
     public HashingStorage setItemWithState(Object key, Object value, ThreadState state,
                     @CachedLanguage PythonLanguage lang,
                     @CachedLibrary(limit = "2") HashingStorageLibrary lib,
-                    @Exclusive @Cached("createBinaryProfile()") ConditionProfile gotState, @Shared("gil") @Cached GilNode gil) {
-        boolean mustRelease = gil.acquire();
-        try {
-            HashingStorage newStore;
-            if (key instanceof String) {
-                newStore = new DynamicObjectStorage(lang);
-            } else {
-                newStore = EconomicMapStorage.create();
-            }
-            if (gotState.profile(state != null)) {
-                lib.setItemWithState(newStore, key, value, state);
-            } else {
-                lib.setItem(newStore, key, value);
-            }
-            return newStore;
-        } finally {
-            gil.release(mustRelease);
+                    @Exclusive @Cached("createBinaryProfile()") ConditionProfile gotState) {
+        HashingStorage newStore;
+        if (key instanceof String) {
+            newStore = new DynamicObjectStorage(lang);
+        } else {
+            newStore = EconomicMapStorage.create();
+        }
+        if (gotState.profile(state != null)) {
+            lib.setItemWithState(newStore, key, value, state);
+        } else {
+            lib.setItem(newStore, key, value);
         }
+        return newStore;
     }
 
     @ExportMessage(limit = "1")
     public HashingStorage delItemWithState(Object key, ThreadState state,
                     @CachedLibrary("key") PythonObjectLibrary lib,
-                    @Exclusive @Cached("createBinaryProfile()") ConditionProfile gotState, @Shared("gil") @Cached GilNode gil) {
-        boolean mustRelease = gil.acquire();
-        try {
-            // must call __hash__ for potential side-effect
-            getHashWithState(key, lib, state, gotState);
-            return this;
-        } finally {
-            gil.release(mustRelease);
-        }
+                    @Exclusive @Cached("createBinaryProfile()") ConditionProfile gotState) {
+        // must call __hash__ for potential side-effect
+        getHashWithState(key, lib, state, gotState);
+        return this;
     }
 
     @Override

graalpython/com.oracle.graal.python/src/com/oracle/graal/python/builtins/objects/common/EconomicMapStorage.java

@@ -53,11 +53,9 @@
 import com.oracle.graal.python.nodes.PGuards;
 import com.oracle.graal.python.nodes.object.IsBuiltinClassProfile;
 import com.oracle.graal.python.nodes.util.CastToJavaStringNode;
-import com.oracle.graal.python.runtime.GilNode;
 import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
 import com.oracle.truffle.api.dsl.Cached;
 import com.oracle.truffle.api.dsl.Cached.Exclusive;
-import com.oracle.truffle.api.dsl.Cached.Shared;
 import com.oracle.truffle.api.dsl.Specialization;
 import com.oracle.truffle.api.library.CachedLibrary;
 import com.oracle.truffle.api.library.ExportLibrary;
@@ -157,13 +155,9 @@ PythonObjectLibrary getPythonObjLib() {
     }
 
     @ExportMessage
-    public int length(@Shared("gil") @Cached GilNode gil) {
-        boolean mustRelease = gil.acquire();
-        try {
-            return size(values);
-        } finally {
-            gil.release(mustRelease);
-        }
+    @Override
+    public int length() {
+        return size(values);
     }
 
     @TruffleBoundary
@@ -174,25 +168,15 @@ private static int size(LinkedHashMap<Object, Object> map) {
     @ExportMessage
     static class GetItemWithState {
         @Specialization
-        static Object getItemString(HashMapStorage self, String key, @SuppressWarnings("unused") ThreadState state, @Shared("gil") @Cached GilNode gil) {
-            boolean mustRelease = gil.acquire();
-            try {
-                return get(self.values, key);
-            } finally {
-                gil.release(mustRelease);
-            }
+        static Object getItemString(HashMapStorage self, String key, @SuppressWarnings("unused") ThreadState state) {
+            return get(self.values, key);
         }
 
         @Specialization(replaces = "getItemString", guards = "isSupportedKey(key, profile)")
         static Object getItem(HashMapStorage self, Object key, @SuppressWarnings("unused") ThreadState state,
                         @Cached CastToJavaStringNode castNode,
-                        @SuppressWarnings("unused") @Cached IsBuiltinClassProfile profile, @Cached GilNode gil) {
-            boolean mustRelease = gil.acquire();
-            try {
-                return get(self.values, castNode.execute(key));
-            } finally {
-                gil.release(mustRelease);
-            }
+                        @SuppressWarnings("unused") @Cached IsBuiltinClassProfile profile) {
+            return get(self.values, castNode.execute(key));
         }
 
         @TruffleBoundary
@@ -204,49 +188,34 @@ private static Object get(LinkedHashMap<Object, Object> values, Object key) {
         static Object getItemNotSupportedKey(@SuppressWarnings("unused") HashMapStorage self, Object key, @SuppressWarnings("unused") ThreadState state,
                         @SuppressWarnings("unused") @Cached IsBuiltinClassProfile profile,
                         @CachedLibrary("key") PythonObjectLibrary lib,
-                        @Exclusive @Cached("createBinaryProfile()") ConditionProfile gotState, @Cached GilNode gil) {
-            boolean mustRelease = gil.acquire();
-            try {
-                // we must still search the map for items that may have the same hash and that may
-                // return true from key.__eq__, we use artificial object with overridden Java level
-                // equals and hashCode methods to perform this search
-                long hash = getHashWithState(key, lib, state, gotState);
-                if (PInt.isIntRange(hash)) {
-                    CustomKey keyObj = new CustomKey(key, (int) hash, state);
-                    return get(self.values, keyObj);
-                }
-                // else the hashes cannot possibly match
-                return null;
-            } finally {
-                gil.release(mustRelease);
+                        @Exclusive @Cached("createBinaryProfile()") ConditionProfile gotState) {
+            // we must still search the map for items that may have the same hash and that may
+            // return true from key.__eq__, we use artificial object with overridden Java level
+            // equals and hashCode methods to perform this search
+            long hash = getHashWithState(key, lib, state, gotState);
+            if (PInt.isIntRange(hash)) {
+                CustomKey keyObj = new CustomKey(key, (int) hash, state);
+                return get(self.values, keyObj);
             }
+            // else the hashes cannot possibly match
+            return null;
         }
     }
 
     @ExportMessage
     static class SetItemWithState {
         @Specialization
-        static HashingStorage setItemString(HashMapStorage self, String key, Object value, @SuppressWarnings("unused") ThreadState state, @Shared("gil") @Cached GilNode gil) {
-            boolean mustRelease = gil.acquire();
-            try {
-                put(self.values, key, value);
-                return self;
-            } finally {
-                gil.release(mustRelease);
-            }
+        static HashingStorage setItemString(HashMapStorage self, String key, Object value, @SuppressWarnings("unused") ThreadState state) {
+            put(self.values, key, value);
+            return self;
         }
 
         @Specialization(replaces = "setItemString", guards = "isSupportedKey(key, profile)")
         static HashingStorage setItem(HashMapStorage self, Object key, Object value, @SuppressWarnings("unused") ThreadState state,
                         @Cached CastToJavaStringNode castNode,
-                        @SuppressWarnings("unused") @Cached IsBuiltinClassProfile profile, @Cached GilNode gil) {
-            boolean mustRelease = gil.acquire();
-            try {
-                put(self.values, castNode.execute(key), value);
-                return self;
-            } finally {
-                gil.release(mustRelease);
-            }
+                        @SuppressWarnings("unused") @Cached IsBuiltinClassProfile profile) {
+            put(self.values, castNode.execute(key), value);
+            return self;
         }
 
         @TruffleBoundary
@@ -258,43 +227,28 @@ private static void put(LinkedHashMap<Object, Object> values, Object key, Object
         static HashingStorage setItemNotSupportedKey(HashMapStorage self, Object key, Object value, @SuppressWarnings("unused") ThreadState state,
                         @SuppressWarnings("unused") @Cached IsBuiltinClassProfile profile,
                         @CachedLibrary("self") HashingStorageLibrary thisLib,
-                        @CachedLibrary(limit = "1") HashingStorageLibrary newLib, @Cached GilNode gil) {
-            boolean mustRelease = gil.acquire();
-            try {
-                HashingStorage newStore = EconomicMapStorage.create(self.length(gil));
-                thisLib.addAllToOther(self, newStore);
-                newLib.setItem(newStore, key, value);
-                return newStore;
-            } finally {
-                gil.release(mustRelease);
-            }
+                        @CachedLibrary(limit = "1") HashingStorageLibrary newLib) {
+            HashingStorage newStore = EconomicMapStorage.create(self.length());
+            thisLib.addAllToOther(self, newStore);
+            newLib.setItem(newStore, key, value);
+            return newStore;
         }
     }
 
     @ExportMessage
     static class DelItemWithState {
         @Specialization
-        static HashingStorage delItemString(HashMapStorage self, String key, @SuppressWarnings("unused") ThreadState state, @Shared("gil") @Cached GilNode gil) {
-            boolean mustRelease = gil.acquire();
-            try {
-                remove(self.values, key);
-                return self;
-            } finally {
-                gil.release(mustRelease);
-            }
+        static HashingStorage delItemString(HashMapStorage self, String key, @SuppressWarnings("unused") ThreadState state) {
+            remove(self.values, key);
+            return self;
         }
 
         @Specialization(replaces = "delItemString", guards = "isSupportedKey(key, profile)")
         static HashingStorage delItem(HashMapStorage self, Object key, @SuppressWarnings("unused") ThreadState state,
                         @Cached CastToJavaStringNode castNode,
-                        @SuppressWarnings("unused") @Cached IsBuiltinClassProfile profile, @Cached GilNode gil) {
-            boolean mustRelease = gil.acquire();
-            try {
-                remove(self.values, castNode.execute(key));
-                return self;
-            } finally {
-                gil.release(mustRelease);
-            }
+                        @SuppressWarnings("unused") @Cached IsBuiltinClassProfile profile) {
+            remove(self.values, castNode.execute(key));
+            return self;
         }
 
         @TruffleBoundary
@@ -306,48 +260,35 @@ private static void remove(LinkedHashMap<Object, Object> values, Object key) {
         static HashingStorage delItemNonSupportedKey(HashMapStorage self, @SuppressWarnings("unused") Object key, @SuppressWarnings("unused") ThreadState state,
                         @SuppressWarnings("unused") @Cached IsBuiltinClassProfile profile,
                         @CachedLibrary("key") PythonObjectLibrary lib,
-                        @Exclusive @Cached("createBinaryProfile()") ConditionProfile gotState, @Cached GilNode gil) {
-            boolean mustRelease = gil.acquire();
-            try {
-                // we must still search the map for items that may have the same hash and that may
-                // return true from key.__eq__, we use artificial object with overridden Java level
-                // equals and hashCode methods to perform this search
-                long hash = getHashWithState(key, lib, state, gotState);
-                if (PInt.isIntRange(hash)) {
-                    CustomKey keyObj = new CustomKey(key, (int) hash, state);
-                    remove(self.values, keyObj);
-                }
-                // else the hashes cannot possibly match
-                return self;
-            } finally {
-                gil.release(mustRelease);
+                        @Exclusive @Cached("createBinaryProfile()") ConditionProfile gotState) {
+            // we must still search the map for items that may have the same hash and that may
+            // return true from key.__eq__, we use artificial object with overridden Java level
+            // equals and hashCode methods to perform this search
+            long hash = getHashWithState(key, lib, state, gotState);
+            if (PInt.isIntRange(hash)) {
+                CustomKey keyObj = new CustomKey(key, (int) hash, state);
+                remove(self.values, keyObj);
             }
+            // else the hashes cannot possibly match
+            return self;
         }
     }
 
     @ExportMessage
-    Object forEachUntyped(ForEachNode<Object> node, Object argIn, @Shared("gil") @Cached GilNode gil) {
-        boolean mustRelease = gil.acquire();
-        try {
-            Object arg = argIn;
-            for (Object key : keys(gil)) {
-                arg = node.execute(key, arg);
-            }
-            return arg;
-        } finally {
-            gil.release(mustRelease);
+    @Override
+    Object forEachUntyped(ForEachNode<Object> node, Object argIn) {
+        Object arg = argIn;
+        for (Object key : keys()) {
+            arg = node.execute(key, arg);
         }
+        return arg;
     }
 
     @ExportMessage
-    public HashingStorage clear(@Shared("gil") @Cached GilNode gil) {
-        boolean mustRelease = gil.acquire();
-        try {
-            clearMap(values);
-            return this;
-        } finally {
-            gil.release(mustRelease);
-        }
+    @Override
+    public HashingStorage clear() {
+        clearMap(values);
+        return this;
     }
 
     @TruffleBoundary
@@ -356,37 +297,25 @@ private static void clearMap(LinkedHashMap<Object, Object> map) {
     }
 
     @ExportMessage
-    public HashingStorage copy(@Shared("gil") @Cached GilNode gil) {
-        boolean mustRelease = gil.acquire();
-        try {
-            return new HashMapStorage(newHashMap(values));
-        } finally {
-            gil.release(mustRelease);
-        }
+    @Override
+    public HashingStorage copy() {
+        return new HashMapStorage(newHashMap(values));
     }
 
     @ExportMessage
-    public HashingStorageIterable<Object> keys(@Shared("gil") @Cached GilNode gil) {
-        boolean mustRelease = gil.acquire();
-        try {
-            return new HashingStorageIterable<>(getKeysIterator(values));
-        } finally {
-            gil.release(mustRelease);
-        }
+    @Override
+    public HashingStorageIterable<Object> keys() {
+        return new HashingStorageIterable<>(getKeysIterator(values));
     }
 
     private static Iterator<Object> getKeysIterator(LinkedHashMap<Object, Object> map) {
         return map.keySet().iterator();
     }
 
     @ExportMessage
-    public HashingStorageIterable<Object> reverseKeys(@Shared("gil") @Cached GilNode gil) {
-        boolean mustRelease = gil.acquire();
-        try {
-            return new HashingStorageIterable<>(getReverseIterator(values));
-        } finally {
-            gil.release(mustRelease);
-        }
+    @Override
+    public HashingStorageIterable<Object> reverseKeys() {
+        return new HashingStorageIterable<>(getReverseIterator(values));
     }
 
     @TruffleBoundary