Commits

Anonymous committed c110243

Resolves OGNL-141. Added runtime optimization for method invocations such that knowledge of whether or not the method needs synchronized invocations because of private access (or whether or not security checks needs to be done) is cached - thus avoiding synchronized blocks altogether on methods that have been invoked at least once already.

  • Participants
  • Parent commits 0d2cd03

Comments (0)

Files changed (3)

   </component>
   <component name="ChangeListManager">
     <list default="true" name="Default" comment="">
-      <change type="MODIFICATION" beforePath="$PROJECT_DIR$/OGNL.iml" afterPath="$PROJECT_DIR$/OGNL.iml" />
       <change type="MODIFICATION" beforePath="$PROJECT_DIR$/OGNL.iws" afterPath="$PROJECT_DIR$/OGNL.iws" />
-      <change type="MODIFICATION" beforePath="$PROJECT_DIR$/OGNL.ipr" afterPath="$PROJECT_DIR$/OGNL.ipr" />
-      <change type="MODIFICATION" beforePath="$PROJECT_DIR$/src/java/ognl/enhance/ExpressionCompiler.java" afterPath="$PROJECT_DIR$/src/java/ognl/enhance/ExpressionCompiler.java" />
+      <change type="MODIFICATION" beforePath="$PROJECT_DIR$/src/java/ognl/OgnlRuntime.java" afterPath="$PROJECT_DIR$/src/java/ognl/OgnlRuntime.java" />
+      <change type="MODIFICATION" beforePath="$PROJECT_DIR$/src/test/java/org/ognl/test/Performance.java" afterPath="$PROJECT_DIR$/src/test/java/org/ognl/test/Performance.java" />
     </list>
   </component>
   <component name="ChangesViewManager" flattened_view="true" show_ignored="false" />
     </subPane>
   </component>
   <component name="FileEditorManager">
-    <leaf />
+    <leaf>
+      <file leaf-file-name="OgnlRuntime.java" pinned="false" current="true" current-in-tab="true">
+        <entry file="file://$PROJECT_DIR$/src/java/ognl/OgnlRuntime.java">
+          <provider selected="true" editor-type-id="text-editor">
+            <state line="820" column="26" selection-start="29711" selection-end="29711" vertical-scroll-proportion="0.45193097">
+              <folding />
+            </state>
+          </provider>
+        </entry>
+      </file>
+    </leaf>
   </component>
   <component name="FindManager">
     <FindUsagesManager>
     <option name="PERFORM_EDIT_IN_BACKGROUND" value="true" />
     <option name="PERFORM_ADD_REMOVE_IN_BACKGROUND" value="true" />
     <option name="FORCE_NON_EMPTY_COMMENT" value="false" />
-    <option name="LAST_COMMIT_MESSAGE" value="Minor improvements to remove redundant expression evaluations during compilation." />
+    <option name="LAST_COMMIT_MESSAGE" value="Resolves OGNL-141.  Added runtime optimization for method invocations such that knowledge of whether or not the method needs synchronized invocations because of private access (or whether or not security checks needs to be done) is cached - thus avoiding synchronized blocks altogether on methods that have been invoked at least once already." />
     <option name="OPTIMIZE_IMPORTS_BEFORE_PROJECT_COMMIT" value="true" />
     <option name="REFORMAT_BEFORE_PROJECT_COMMIT" value="false" />
     <option name="REFORMAT_BEFORE_FILE_COMMIT" value="false" />
     <option name="UPDATE_GROUP_BY_CHANGELIST" value="false" />
     <option name="SHOW_FILE_HISTORY_AS_TREE" value="false" />
     <option name="FILE_HISTORY_SPLITTER_PROPORTION" value="0.6" />
-    <MESSAGE value="Fixes OGNL-106.  Wasn't checking for null in ASTMethod.&#10;&#10;Also refactored exception catching logic so that more exceptions aren't needlessly added on to the exception stack when catching Throwables." />
-    <MESSAGE value="Fixes OGNL-108. ASTInstanceof wasn't setting the context type before returning." />
     <MESSAGE value="Fixes OGNL-110.  Numeric literals were being added to non literal expressions. (such as property getters)" />
     <MESSAGE value="Slightly better exception reporting for methods that can't be found." />
     <MESSAGE value="Fixes OGNL-112.  OgnlRuntime was incorrectly returning reader methods for simple property getters with parameters when non parameter methods with the same name should have been preferred in the method finding loop." />
     <MESSAGE value="Testing list expressions." />
     <MESSAGE value="Reduced some overhead with unneccesarily getting values from expressions when compiling." />
     <MESSAGE value="Minor improvements to remove redundant expression evaluations during compilation." />
+    <MESSAGE value="Resolves OGNL-141.  Added runtime optimization for method invocations such that knowledge of whether or not the method" />
+    <MESSAGE value="Resolves OGNL-141.  Added runtime optimization for method invocations such that knowledge of whether or not the method needs synchronized invocations because of private access (or whether or not security checks needs to be done) is cached - thus avoiding synchronized blocks altogether on methods that have been invoked at least once already." />
   </component>
   <component name="VssConfiguration">
     <option name="CLIENT_PATH" value="" />
     <option name="myLastEditedConfigurable" />
   </component>
   <component name="editorHistoryManager">
-    <entry file="file://$PROJECT_DIR$/src/java/ognl/ASTAdd.java">
-      <provider selected="true" editor-type-id="text-editor">
-        <state line="118" column="5" selection-start="4496" selection-end="4496" vertical-scroll-proportion="1.5243446">
-          <folding />
-        </state>
-      </provider>
-    </entry>
-    <entry file="file://$PROJECT_DIR$/src/java/ognl/ASTStaticField.java">
-      <provider selected="true" editor-type-id="text-editor">
-        <state line="180" column="43" selection-start="5754" selection-end="5785" vertical-scroll-proportion="0.6741573">
-          <folding />
-        </state>
-      </provider>
-    </entry>
-    <entry file="file://$PROJECT_DIR$/src/test/java/org/ognl/test/NullStringCatenationTest.java">
-      <provider selected="true" editor-type-id="text-editor">
-        <state line="55" column="6" selection-start="2895" selection-end="2895" vertical-scroll-proportion="0.4681648">
-          <folding />
-        </state>
-      </provider>
-    </entry>
-    <entry file="file://$PROJECT_DIR$/src/java/ognl/MethodAccessor.java">
-      <provider selected="true" editor-type-id="text-editor">
-        <state line="44" column="7" selection-start="2136" selection-end="2136" vertical-scroll-proportion="0.18488085">
-          <folding />
-        </state>
-      </provider>
-    </entry>
-    <entry file="file://$PROJECT_DIR$/src/java/ognl/ObjectMethodAccessor.java">
-      <provider selected="true" editor-type-id="text-editor">
-        <state line="51" column="38" selection-start="2422" selection-end="2422" vertical-scroll-proportion="0.39325842">
-          <folding />
-        </state>
-      </provider>
-    </entry>
-    <entry file="file://$PROJECT_DIR$/src/java/ognl/ASTStaticMethod.java">
-      <provider selected="true" editor-type-id="text-editor">
-        <state line="118" column="48" selection-start="4062" selection-end="4062" vertical-scroll-proportion="0.25093633">
-          <folding />
-        </state>
-      </provider>
-    </entry>
-    <entry file="file://$PROJECT_DIR$/src/test/java/org/ognl/test/MemberAccessTest.java">
-      <provider selected="true" editor-type-id="text-editor">
-        <state line="51" column="6" selection-start="2528" selection-end="2528" vertical-scroll-proportion="0.28089887">
-          <folding />
-        </state>
-      </provider>
-    </entry>
-    <entry file="file://$PROJECT_DIR$/src/java/ognl/DefaultMemberAccess.java">
-      <provider selected="true" editor-type-id="text-editor">
-        <state line="133" column="54" selection-start="4957" selection-end="4957" vertical-scroll-proportion="0.716516">
-          <folding />
-        </state>
-      </provider>
-    </entry>
     <entry file="file://$PROJECT_DIR$/src/test/java/org/ognl/test/ArrayCreationTest.java">
       <provider selected="true" editor-type-id="text-editor">
         <state line="53" column="19" selection-start="2705" selection-end="2705" vertical-scroll-proportion="0.24650781">
         </state>
       </provider>
     </entry>
-    <entry file="file://$PROJECT_DIR$/src/java/ognl/OgnlRuntime.java">
-      <provider selected="true" editor-type-id="text-editor">
-        <state line="1869" column="0" selection-start="69574" selection-end="69574" vertical-scroll-proportion="0.23171733">
-          <folding />
-        </state>
-      </provider>
-    </entry>
     <entry file="file://$PROJECT_DIR$/src/test/java/org/ognl/test/objects/Simple.java">
       <provider selected="true" editor-type-id="text-editor">
         <state line="192" column="17" selection-start="4671" selection-end="4671" vertical-scroll-proportion="1.4264585">
         </state>
       </provider>
     </entry>
+    <entry file="jar:///usr/local/jdk1.5.0_12/src.zip!/java/security/BasicPermission.java">
+      <provider selected="true" editor-type-id="text-editor">
+        <state line="28" column="24" selection-start="845" selection-end="845" vertical-scroll-proportion="0.14790468">
+          <folding />
+        </state>
+      </provider>
+    </entry>
+    <entry file="file://$PROJECT_DIR$/src/java/ognl/OgnlInvokePermission.java">
+      <provider selected="true" editor-type-id="text-editor">
+        <state line="43" column="52" selection-start="2258" selection-end="2258" vertical-scroll-proportion="0.17255546">
+          <folding />
+        </state>
+      </provider>
+    </entry>
+    <entry file="jar:///usr/local/jdk1.5.0_12/src.zip!/java/security/Permission.java">
+      <provider selected="true" editor-type-id="text-editor">
+        <state line="173" column="7" selection-start="6052" selection-end="6052" vertical-scroll-proportion="0.40838125">
+          <folding />
+        </state>
+      </provider>
+    </entry>
+    <entry file="file://$PROJECT_DIR$/src/java/ognl/internal/ClassCacheImpl.java">
+      <provider selected="true" editor-type-id="text-editor">
+        <state line="23" column="5" selection-start="606" selection-end="606" vertical-scroll-proportion="0.2588332">
+          <folding />
+        </state>
+      </provider>
+    </entry>
+    <entry file="file://$PROJECT_DIR$/src/java/ognl/ObjectArrayPool.java">
+      <provider selected="true" editor-type-id="text-editor">
+        <state line="34" column="23" selection-start="1802" selection-end="1817" vertical-scroll-proportion="0.061626952">
+          <folding />
+        </state>
+      </provider>
+    </entry>
+    <entry file="file://$PROJECT_DIR$/src/java/ognl/IntHashMap.java">
+      <provider selected="true" editor-type-id="text-editor">
+        <state line="141" column="0" selection-start="5050" selection-end="5050" vertical-scroll-proportion="0.45603943">
+          <folding />
+        </state>
+      </provider>
+    </entry>
+    <entry file="file://$PROJECT_DIR$/src/test/java/org/ognl/test/Performance.java">
+      <provider selected="true" editor-type-id="text-editor">
+        <state line="140" column="13" selection-start="5286" selection-end="5286" vertical-scroll-proportion="0.17255546">
+          <folding />
+        </state>
+      </provider>
+    </entry>
+    <entry file="jar:///usr/local/jdk1.5.0_12/src.zip!/java/lang/SecurityManager.java">
+      <provider selected="true" editor-type-id="text-editor">
+        <state line="530" column="16" selection-start="21132" selection-end="21132" vertical-scroll-proportion="0.33278555">
+          <folding />
+        </state>
+      </provider>
+    </entry>
+    <entry file="file://$PROJECT_DIR$/src/java/ognl/OgnlRuntime.java">
+      <provider selected="true" editor-type-id="text-editor">
+        <state line="820" column="26" selection-start="29711" selection-end="29711" vertical-scroll-proportion="0.45193097">
+          <folding />
+        </state>
+      </provider>
+    </entry>
   </component>
   <component name="testng.defaultConfiguration">
     <outputDirectory />

src/java/ognl/OgnlRuntime.java

     static final EvaluationPool _evaluationPool = new EvaluationPool();
     static final ObjectArrayPool _objectArrayPool = new ObjectArrayPool();
 
+    static final IntHashMap _methodAccessCache = new IntHashMap();
+    static final IntHashMap _methodPermCache = new IntHashMap();
+
     static ClassCacheInspector _cacheInspector;
 
     /**
         _superclasses.clear();
         _declaredMethods[0].clear();
         _declaredMethods[1].clear();
+        _methodAccessCache.clear();
     }
 
     /**
             ParameterizedType param = (ParameterizedType)type.getGenericSuperclass();
             Type[] genTypes = m.getGenericParameterTypes();
             TypeVariable[] declaredTypes = m.getDeclaringClass().getTypeParameters();
-            
+
             types = new Class[genTypes.length];
 
             typeSearch:
 
             return types;
         }
-    } 
+    }
 
     static Class resolveType(ParameterizedType param, TypeVariable var, TypeVariable[] declaredTypes)
     {
     public static Object invokeMethod(Object target, Method method, Object[] argsArray)
             throws InvocationTargetException, IllegalAccessException
     {
+        boolean syncInvoke = false;
+        boolean checkPermission = false;
+        int mHash = method.hashCode();
+
+        // only synchronize method invocation if it actually requires it
+
+        synchronized(method) {
+            if (_methodAccessCache.get(mHash) == null
+                || _methodAccessCache.get(mHash) == Boolean.TRUE) {
+                syncInvoke = true;
+            }
+
+            if (_securityManager != null && _methodPermCache.get(mHash) == null
+                || _methodPermCache.get(mHash) == Boolean.FALSE) {
+                checkPermission = true;
+            }
+        }
+
         Object result;
         boolean wasAccessible = true;
 
-        synchronized(method) {
-
-            if (_securityManager != null)
+        if (syncInvoke)
+        {
+            synchronized(method)
             {
-                try {
+                if (checkPermission)
+                {
+                    try
+                    {
+                        _securityManager.checkPermission(getPermission(method));
+                        _methodPermCache.put(mHash, Boolean.TRUE);
+                    } catch (SecurityException ex) {
+                        _methodPermCache.put(mHash, Boolean.FALSE);
+                        throw new IllegalAccessException("Method [" + method + "] cannot be accessed.");
+                    }
+                }
+
+                if (!Modifier.isPublic(method.getModifiers()) || !Modifier.isPublic(method.getDeclaringClass().getModifiers()))
+                {
+                    if (!(wasAccessible = ((AccessibleObject) method).isAccessible()))
+                    {
+                        ((AccessibleObject) method).setAccessible(true);
+                        _methodAccessCache.put(mHash, Boolean.TRUE);
+                    } else
+                    {
+                        _methodAccessCache.put(mHash, Boolean.FALSE);
+                    }
+                } else
+                {
+                    _methodAccessCache.put(mHash, Boolean.FALSE);
+                }
+
+                result = method.invoke(target, argsArray);
+
+                if (!wasAccessible)
+                {
+                    ((AccessibleObject) method).setAccessible(false);
+                }
+            }
+        } else
+        {
+            if (checkPermission)
+            {
+                try
+                {
                     _securityManager.checkPermission(getPermission(method));
+                    _methodPermCache.put(mHash, Boolean.TRUE);
                 } catch (SecurityException ex) {
+                    _methodPermCache.put(mHash, Boolean.FALSE);
                     throw new IllegalAccessException("Method [" + method + "] cannot be accessed.");
                 }
             }
 
-            if (!Modifier.isPublic(method.getModifiers()) || !Modifier.isPublic(method.getDeclaringClass().getModifiers()))
-            {
-                if (!(wasAccessible = ((AccessibleObject) method).isAccessible()))
-                {
-                    ((AccessibleObject) method).setAccessible(true);
-                }
-            }
-
             result = method.invoke(target, argsArray);
-
-            if (!wasAccessible)
-            {
-                ((AccessibleObject) method).setAccessible(false);
-            }
         }
 
         return result;
                 if (!result && classes[index].isArray()) {
                     result = isTypeCompatible(args[index], classes[index].getComponentType());
                 }
-
-                //result = isTypeCompatible(args[index], classes[index]) || classes[index].isArray();
             }
         } else {
             for (int index = 0, count = args.length; result && (index < count); ++index) {
                 {
                     typeClass = (Class)source;
                 }
-                
-                Class[] mParameterTypes = findParameterTypes(typeClass, m);//getParameterTypes(m);
+
+                Class[] mParameterTypes = findParameterTypes(typeClass, m);
 
                 if (areArgsCompatible(args, mParameterTypes, m)
                     && ((result == null) || isMoreSpecific(mParameterTypes, resultParameterTypes)))
         ClassCache cache = (staticMethods ? _staticMethodCache : _instanceMethodCache);
         Map result;
 
-        synchronized (cache) {
-            if ((result = (Map) cache.get(targetClass)) == null) {
+        synchronized (cache)
+        {
+            if ((result = (Map) cache.get(targetClass)) == null)
+            {
                 cache.put(targetClass, result = new HashMap(23));
-                for (Class c = targetClass; c != null; c = c.getSuperclass()) {
+
+                for (Class c = targetClass; c != null; c = c.getSuperclass())
+                {
                     Method[] ma = c.getDeclaredMethods();
 
-                    for (int i = 0, icount = ma.length; i < icount; i++) {
-                        if (Modifier.isStatic(ma[i].getModifiers()) == staticMethods) {
+                    for (int i = 0, icount = ma.length; i < icount; i++)
+                    {
+                        if (Modifier.isStatic(ma[i].getModifiers()) == staticMethods)
+                        {
                             List ml = (List) result.get(ma[i].getName());
 
                             if (ml == null)
                                 result.put(ma[i].getName(), ml = new ArrayList());
+
                             ml.add(ma[i]);
                         }
                     }
                 }
             }
         }
+
         return result;
     }
 

src/test/java/org/ognl/test/Performance.java

         try {
             Performance[] tests = new Performance[] {
                     new Performance("Constant", "100 + 20 * 5", "testConstantExpression"),
-                    new Performance("Constant", "100 + 20 * 5", "testConstantExpression", true),
+                    //new Performance("Constant", "100 + 20 * 5", "testConstantExpression", false),
                     new Performance("Single Property", "bean2", "testSinglePropertyExpression"),
-                    new Performance("Single Property", "bean2", "testSinglePropertyExpression", true),
                     new Performance("Property Navigation", "bean2.bean3.value", "testPropertyNavigationExpression"),
-                    new Performance("Property Navigation", "bean2.bean3.value", "testPropertyNavigationExpression", true),
                     /*new Performance("Property Setting with context key", "bean2.bean3.nullValue", "testPropertyNavigationSetting"),
                     new Performance("Property Setting with context key", "bean2.bean3.nullValue", "testPropertyNavigationSetting", true), */                    
                     new Performance("Property Navigation and Comparison", "bean2.bean3.value <= 24",
                                     "testPropertyNavigationAndComparisonExpression"),
-                    new Performance("Property Navigation and Comparison", "bean2.bean3.value <= 24",
-                                    "testPropertyNavigationAndComparisonExpression", true),
                     /* new Performance("Property Navigation with Indexed Access", "bean2.bean3.indexedValue[25]",
                                     "testIndexedPropertyNavigationExpression"),
                     new Performance("Property Navigation with Indexed Access", "bean2.bean3.indexedValue[25]",
                                     "testIndexedPropertyNavigationExpression", true), */
                     new Performance("Property Navigation with Map Access", "bean2.bean3.map['foo']",
                                     "testPropertyNavigationWithMapExpression"),
-                    new Performance("Property Navigation with Map Access", "bean2.bean3.map['foo']",
-                                    "testPropertyNavigationWithMapExpression", true),
                     /* new Performance("Property Navigation with Map value set", "bean2.bean3.map['foo']",
                                     "testPropertyNavigationWithMapSetting"),
                     new Performance("Property Navigation with Map value set", "bean2.bean3.map['foo']",