[Jython-checkins] jython: Make sure JyAttr (*Attr) methods are synchronizing on the objects they serve

darjus.loktevic jython-checkins at python.org
Sat Jan 9 07:25:32 EST 2016


https://hg.python.org/jython/rev/724ea1e797f2
changeset:   7861:724ea1e797f2
user:        Darjus Loktevic <darjus at gmail.com>
date:        Sat Jan 09 21:52:32 2016 +1100
summary:
  Make sure JyAttr (*Attr) methods are synchronizing on the objects they serve and not themselves. Fixes a performance regression #2452
Additional small refactorings and branch optmizations after checking with jitwatch.

files:
  src/org/python/core/JyAttribute.java |  187 +++++++-------
  src/org/python/core/PyJavaType.java  |    7 +-
  src/org/python/core/PyObject.java    |   21 +-
  3 files changed, 109 insertions(+), 106 deletions(-)


diff --git a/src/org/python/core/JyAttribute.java b/src/org/python/core/JyAttribute.java
--- a/src/org/python/core/JyAttribute.java
+++ b/src/org/python/core/JyAttribute.java
@@ -184,18 +184,8 @@
      * Checks whether the given {@link org.python.core.PyObject}
      * has an attribute of the given type attached.
      */
-    public static synchronized boolean hasAttr(PyObject ob, byte attr_type) {
-        if (ob.attributes == null) {
-            return false;
-        }
-        if (!(ob.attributes instanceof JyAttribute)) {
-            return attr_type == JAVA_PROXY_ATTR;
-        }
-        JyAttribute att = (JyAttribute) ob.attributes;
-        while (att != null && att.attr_type < attr_type) {
-            att = att.getNext();
-        }
-        return att != null && att.attr_type == attr_type;
+    public static boolean hasAttr(PyObject ob, byte attr_type) {
+        return getAttr(ob, attr_type) != null;
     }
 
     /**
@@ -203,18 +193,17 @@
      * {@link org.python.core.PyObject}.
      * If no attribute of the given type is attached, null is returned.
      */
-    public static synchronized Object getAttr(PyObject ob, byte attr_type) {
-        if (ob.attributes == null) {
-            return null;
-        }
-        if (!(ob.attributes instanceof JyAttribute)) {
+    public static Object getAttr(PyObject ob, byte attr_type) {
+        synchronized (ob) {
+            if (ob.attributes instanceof JyAttribute) {
+                JyAttribute att = (JyAttribute) ob.attributes;
+                while (att != null && att.attr_type < attr_type) {
+                    att = att.getNext();
+                }
+                return att != null && att.attr_type == attr_type ? att.getValue() : null;
+            }
             return attr_type == JAVA_PROXY_ATTR ? ob.attributes : null;
         }
-        JyAttribute att = (JyAttribute) ob.attributes;
-        while (att != null && att.attr_type < attr_type) {
-            att = att.getNext();
-        }
-        return att != null && att.attr_type == attr_type ? att.getValue() : null;
     }
 
     /**
@@ -222,20 +211,22 @@
      * given object to the given stream.
      * (Intended for debugging)
      */
-    public static synchronized void debugPrintAttributes(PyObject o, java.io.PrintStream out) {
-        out.println("debugPrintAttributes of "+System.identityHashCode(o)+":");
-        if (o.attributes == null) {
-            out.println("null");
-        } else if (!(o.attributes instanceof JyAttribute)) {
-            out.println("only javaProxy");
-        } else {
-            JyAttribute att = (JyAttribute) o.attributes;
-            while (att != null) {
-                out.println("att type: "+att.attr_type+" value: "+att.getValue());
-                att = att.getNext();
+    public static void debugPrintAttributes(PyObject o, java.io.PrintStream out) {
+        synchronized (o) {
+            out.println("debugPrintAttributes of " + System.identityHashCode(o) + ":");
+            if (o.attributes == null) {
+                out.println("null");
+            } else if (!(o.attributes instanceof JyAttribute)) {
+                out.println("only javaProxy");
+            } else {
+                JyAttribute att = (JyAttribute) o.attributes;
+                while (att != null) {
+                    out.println("att type: " + att.attr_type + " value: " + att.getValue());
+                    att = att.getNext();
+                }
             }
+            out.println("debugPrintAttributes done");
         }
-        out.println("debugPrintAttributes done");
     }
 
     /**
@@ -243,51 +234,53 @@
      * If no corresponding attribute exists yet, one is created. If {@value == null},
      * the attribute is removed (if it existed at all).
      */
-    public static synchronized void setAttr(PyObject ob, byte attr_type, Object value) {
-        if (value == null) {
-            delAttr(ob, attr_type);
-        } else {
-            if (ob.attributes == null) {
-                if (attr_type == JyAttribute.JAVA_PROXY_ATTR) {
-                    ob.attributes = value;
+    public static void setAttr(PyObject ob, byte attr_type, Object value) {
+        synchronized (ob) {
+            if (value == null) {
+                delAttr(ob, attr_type);
+            } else {
+                if (ob.attributes == null) {
+                    if (attr_type == JyAttribute.JAVA_PROXY_ATTR) {
+                        ob.attributes = value;
+                    } else {
+                        ob.attributes = attr_type < 0 ?
+                                new AttributeLink(attr_type, value) :
+                                new TransientAttributeLink(attr_type, value);
+                    }
+                } else if (!(ob.attributes instanceof JyAttribute)) {
+                    if (attr_type == JyAttribute.JAVA_PROXY_ATTR) {
+                        ob.attributes = value;
+                    } else {
+                        ob.attributes = new AttributeLink(JyAttribute.JAVA_PROXY_ATTR, ob.attributes);
+                        ((JyAttribute) ob.attributes).setNext(attr_type < 0 ?
+                                new AttributeLink(attr_type, value) :
+                                new TransientAttributeLink(attr_type, value));
+                    }
                 } else {
-                    ob.attributes = attr_type < 0 ?
-                        new AttributeLink(attr_type, value) :
-                        new TransientAttributeLink(attr_type, value);
-                }
-            } else if (!(ob.attributes instanceof JyAttribute)) {
-                if (attr_type == JyAttribute.JAVA_PROXY_ATTR) {
-                    ob.attributes = value;
-                } else {
-                    ob.attributes = new AttributeLink(JyAttribute.JAVA_PROXY_ATTR, ob.attributes);
-                    ((JyAttribute) ob.attributes).setNext(attr_type < 0 ?
-                        new AttributeLink(attr_type, value) :
-                           new TransientAttributeLink(attr_type, value));
-                }
-            } else {
-                JyAttribute att = (JyAttribute) ob.attributes;
-                if (att.attr_type > attr_type) {
-                    JyAttribute newAtt = attr_type < 0 ?
-                        new AttributeLink(attr_type, value) :
-                        new TransientAttributeLink(attr_type, value);
-                    newAtt.setNext(att);
-                    ob.attributes = newAtt;
-                } else {
-                    while (att.getNext() != null && att.getNext().attr_type <= attr_type) {
-                        att = att.getNext();
-                    }
-                    if (att.attr_type == attr_type) {
-                        att.setValue(value);
-                    } else if (att.getNext() == null) {
-                        att.setNext(attr_type < 0 ?
-                            new AttributeLink(attr_type, value) :
-                            new TransientAttributeLink(attr_type, value));
+                    JyAttribute att = (JyAttribute) ob.attributes;
+                    if (att.attr_type > attr_type) {
+                        JyAttribute newAtt = attr_type < 0 ?
+                                new AttributeLink(attr_type, value) :
+                                new TransientAttributeLink(attr_type, value);
+                        newAtt.setNext(att);
+                        ob.attributes = newAtt;
                     } else {
-                        JyAttribute newAtt = attr_type < 0 ?
-                            new AttributeLink(attr_type, value) :
-                            new TransientAttributeLink(attr_type, value);
-                        newAtt.setNext(att.getNext());
-                        att.setNext(newAtt);
+                        while (att.getNext() != null && att.getNext().attr_type <= attr_type) {
+                            att = att.getNext();
+                        }
+                        if (att.attr_type == attr_type) {
+                            att.setValue(value);
+                        } else if (att.getNext() == null) {
+                            att.setNext(attr_type < 0 ?
+                                    new AttributeLink(attr_type, value) :
+                                    new TransientAttributeLink(attr_type, value));
+                        } else {
+                            JyAttribute newAtt = attr_type < 0 ?
+                                    new AttributeLink(attr_type, value) :
+                                    new TransientAttributeLink(attr_type, value);
+                            newAtt.setNext(att.getNext());
+                            att.setNext(newAtt);
+                        }
                     }
                 }
             }
@@ -299,27 +292,29 @@
      * (if it existed at all). This is equivalent to calling
      * {@code setAttr(ob, attr_type, null)}.
      */
-    public static synchronized void delAttr(PyObject ob, byte attr_type) {
-        if (ob.attributes == null) {
-            return;
-        } else if (attr_type == JAVA_PROXY_ATTR && !(ob.attributes instanceof JyAttribute)) {
-            ob.attributes = null;
-        }
-        JyAttribute att = (JyAttribute) ob.attributes;
-        if (att.attr_type == attr_type) {
-            ob.attributes = att.getNext();
-        } else {
-            while (att.getNext() != null && att.getNext().attr_type < attr_type) {
-                att = att.getNext();
+    public static void delAttr(PyObject ob, byte attr_type) {
+        synchronized (ob) {
+            if (ob.attributes == null) {
+                return;
+            } else if (attr_type == JAVA_PROXY_ATTR && !(ob.attributes instanceof JyAttribute)) {
+                ob.attributes = null;
             }
-            if (att.getNext() != null && att.getNext().attr_type == attr_type) {
-                att.setNext(att.getNext().getNext());
+            JyAttribute att = (JyAttribute) ob.attributes;
+            if (att.attr_type == attr_type) {
+                ob.attributes = att.getNext();
+            } else {
+                while (att.getNext() != null && att.getNext().attr_type < attr_type) {
+                    att = att.getNext();
+                }
+                if (att.getNext() != null && att.getNext().attr_type == attr_type) {
+                    att.setNext(att.getNext().getNext());
+                }
             }
-        }
-        if (ob.attributes != null) {
-            att = (JyAttribute) ob.attributes;
-            if (att.getNext() == null && att.attr_type == JyAttribute.JAVA_PROXY_ATTR) {
-                ob.attributes = att.getValue();
+            if (ob.attributes != null) {
+                att = (JyAttribute) ob.attributes;
+                if (att.getNext() == null && att.attr_type == JyAttribute.JAVA_PROXY_ATTR) {
+                    ob.attributes = att.getValue();
+                }
             }
         }
     }
diff --git a/src/org/python/core/PyJavaType.java b/src/org/python/core/PyJavaType.java
--- a/src/org/python/core/PyJavaType.java
+++ b/src/org/python/core/PyJavaType.java
@@ -253,11 +253,14 @@
                 }
                 visibleBases.add(PyType.fromClassSkippingInners(iface, needsInners));
             }
-            if (JyAttribute.getAttr(this, JyAttribute.JAVA_PROXY_ATTR) == Object.class) {
+
+            Object javaProxy = JyAttribute.getAttr(this, JyAttribute.JAVA_PROXY_ATTR);
+
+            if (javaProxy == Object.class) {
                 base = PyType.fromClassSkippingInners(PyObject.class, needsInners);
             } else if(baseClass == null) {
                 base = PyType.fromClassSkippingInners(Object.class, needsInners);
-            }else if (JyAttribute.getAttr(this, JyAttribute.JAVA_PROXY_ATTR) == Class.class) {
+            } else if (javaProxy == Class.class) {
                 base = PyType.fromClassSkippingInners(PyType.class, needsInners);
             } else {
                 base = PyType.fromClassSkippingInners(baseClass, needsInners);
diff --git a/src/org/python/core/PyObject.java b/src/org/python/core/PyObject.java
--- a/src/org/python/core/PyObject.java
+++ b/src/org/python/core/PyObject.java
@@ -336,8 +336,9 @@
      * @param c the Class to convert this <code>PyObject</code> to.
      **/
     public Object __tojava__(Class<?> c) {
-        if ((c == Object.class || c == Serializable.class) && getJavaProxy() != null) {
-            return JyAttribute.getAttr(this, JyAttribute.JAVA_PROXY_ATTR);
+        Object proxy = getJavaProxy();
+        if ((c == Object.class || c == Serializable.class) && proxy != null) {
+            return proxy;
         }
         if (c.isInstance(this)) {
             return this;
@@ -348,8 +349,8 @@
                 c = tmp;
             }
         }
-        if (c.isInstance(getJavaProxy())) {
-            return JyAttribute.getAttr(this, JyAttribute.JAVA_PROXY_ATTR);
+        if (c.isInstance(proxy)) {
+            return proxy;
         }
 
         // convert faux floats
@@ -382,11 +383,15 @@
         return Py.NoConversion;
     }
 
-    protected synchronized Object getJavaProxy() {
-        if (!JyAttribute.hasAttr(this, JyAttribute.JAVA_PROXY_ATTR)) {
-            proxyInit();
+    protected Object getJavaProxy() {
+        Object ob = JyAttribute.getAttr(this, JyAttribute.JAVA_PROXY_ATTR);
+        if (ob == null) {
+            synchronized (this) {
+                proxyInit();
+                ob = JyAttribute.getAttr(this, JyAttribute.JAVA_PROXY_ATTR);
+            }
         }
-        return JyAttribute.getAttr(this, JyAttribute.JAVA_PROXY_ATTR);
+        return ob;
     }
 
     /**

-- 
Repository URL: https://hg.python.org/jython


More information about the Jython-checkins mailing list