Reduce the number of times ThreadLocal is accessed on critical path

impl/src/main/java/org/jboss/weld/bean/proxy/CombinedInterceptorAndDecoratorStackMethodHandler.java

@@ -1,23 +1,23 @@
 package org.jboss.weld.bean.proxy;
 
-import static org.jboss.weld.bean.proxy.InterceptionDecorationContext.endInterceptorContext;
-
 import java.io.Serializable;
 import java.lang.reflect.Method;
 
+import org.jboss.weld.bean.proxy.InterceptionDecorationContext.Stack;
 import org.jboss.weld.exceptions.UnsupportedOperationException;
+import org.jboss.weld.interceptor.proxy.InterceptorMethodHandler;
 import org.jboss.weld.util.reflection.Reflections;
 
 /**
  * A method handler that wraps the invocation of interceptors and decorators.
  *
  * @author Marius Bogoevici
  */
-public class CombinedInterceptorAndDecoratorStackMethodHandler implements MethodHandler, Serializable {
+public class CombinedInterceptorAndDecoratorStackMethodHandler implements StackAwareMethodHandler, Serializable {
 
     public static final CombinedInterceptorAndDecoratorStackMethodHandler NULL_INSTANCE = new CombinedInterceptorAndDecoratorStackMethodHandler() {
         @Override
-        public void setInterceptorMethodHandler(MethodHandler interceptorMethodHandler) {
+        public void setInterceptorMethodHandler(InterceptorMethodHandler interceptorMethodHandler) {
             throw new UnsupportedOperationException();
         }
 
@@ -32,32 +32,40 @@ public Object invoke(Object self, Method thisMethod, Method proceed, Object[] ar
         }
     };
 
-    private MethodHandler interceptorMethodHandler;
+    private InterceptorMethodHandler interceptorMethodHandler;
 
     private Object outerDecorator;
 
-    public void setInterceptorMethodHandler(MethodHandler interceptorMethodHandler) {
+    public void setInterceptorMethodHandler(InterceptorMethodHandler interceptorMethodHandler) {
         this.interceptorMethodHandler = interceptorMethodHandler;
     }
 
     public void setOuterDecorator(Object outerDecorator) {
         this.outerDecorator = outerDecorator;
     }
 
+    @Override
     public Object invoke(Object self, Method thisMethod, Method proceed, Object[] args) throws Throwable {
+        return invoke(null, self, thisMethod, proceed, args);
+    }
 
-        if (InterceptionDecorationContext.startIfNotOnTop(this)) {
+    @Override
+    public Object invoke(Stack stack, Object self, Method thisMethod, Method proceed, Object[] args) throws Throwable {
+        if (stack == null) {
+            stack = InterceptionDecorationContext.getStack();
+        }
+        if (stack.startIfNotOnTop(this)) {
             try {
                 if (interceptorMethodHandler != null) {
                     if (proceed != null) {
                         if (outerDecorator == null) {
                             // use WeldSubclass.method$$super() as proceed
-                            return this.interceptorMethodHandler.invoke(self, thisMethod, proceed, args);
+                            return this.interceptorMethodHandler.invoke(stack, self, thisMethod, proceed, args);
                         } else {
-                            return this.interceptorMethodHandler.invoke(outerDecorator, thisMethod, thisMethod, args);
+                            return this.interceptorMethodHandler.invoke(stack, outerDecorator, thisMethod, thisMethod, args);
                         }
                     } else {
-                        return this.interceptorMethodHandler.invoke(self, thisMethod, null, args);
+                        return this.interceptorMethodHandler.invoke(stack, self, thisMethod, null, args);
                     }
                 } else {
                     if (outerDecorator != null) {
@@ -66,14 +74,14 @@ public Object invoke(Object self, Method thisMethod, Method proceed, Object[] ar
                     }
                 }
             } finally {
-                endInterceptorContext();
+                stack.end();
             }
         }
         SecurityActions.ensureAccessible(proceed);
         return Reflections.invokeAndUnwrap(self, proceed, args);
     }
 
-    public MethodHandler getInterceptorMethodHandler() {
+    public InterceptorMethodHandler getInterceptorMethodHandler() {
         return interceptorMethodHandler;
     }
 
@@ -84,4 +92,8 @@ public Object getOuterDecorator() {
     public boolean isDisabledHandler() {
         return this == InterceptionDecorationContext.peekIfNotEmpty();
     }
+
+    public boolean isDisabledHandler(Stack stack) {
+        return this == stack.peek();
+    }
 }

impl/src/main/java/org/jboss/weld/bean/proxy/InterceptedSubclassFactory.java

@@ -17,6 +17,8 @@
 
 package org.jboss.weld.bean.proxy;
 
+import static org.jboss.weld.util.bytecode.DescriptorUtils.classToStringRepresentation;
+
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.lang.reflect.Type;
@@ -34,6 +36,7 @@
 import org.jboss.classfilewriter.code.CodeAttribute;
 import org.jboss.weld.annotated.enhanced.MethodSignature;
 import org.jboss.weld.annotated.enhanced.jlr.MethodSignatureImpl;
+import org.jboss.weld.bean.proxy.InterceptionDecorationContext.Stack;
 import org.jboss.weld.exceptions.WeldException;
 import org.jboss.weld.interceptor.proxy.LifecycleMixin;
 import org.jboss.weld.interceptor.util.proxy.TargetInstanceProxy;
@@ -57,6 +60,7 @@ public class InterceptedSubclassFactory<T> extends ProxyFactory<T> {
     private static final String SUPER_DELEGATE_SUFFIX = "$$super";
 
     private static final String COMBINED_INTERCEPTOR_AND_DECORATOR_STACK_METHOD_HANDLER_CLASS_NAME = CombinedInterceptorAndDecoratorStackMethodHandler.class.getName();
+    private static final String[] INVOKE_METHOD_PARAMETERS = new String[] { classToStringRepresentation(Stack.class), LJAVA_LANG_OBJECT, LJAVA_LANG_REFLECT_METHOD, LJAVA_LANG_REFLECT_METHOD, "[" + LJAVA_LANG_OBJECT  };
 
     private final Set<MethodSignature> enhancedMethodSignatures;
     private final Set<MethodSignature> interceptedMethodSignatures;
@@ -268,24 +272,34 @@ protected static void invokeMethodHandler(ClassMethod method, MethodInformation
         final CodeAttribute b = method.getCodeAttribute();
         b.aload(0);
         b.getfield(method.getClassFile().getName(), METHOD_HANDLER_FIELD_NAME, DescriptorUtils.classToStringRepresentation(MethodHandler.class));
+        b.checkcast(StackAwareMethodHandler.class.getName());
 
         // this is a self invocation optimisation
         // test to see if this is a self invocation, and if so invokespecial the
         // superclass method directly
         if (addProceed) {
             b.dup();
             b.checkcast(COMBINED_INTERCEPTOR_AND_DECORATOR_STACK_METHOD_HANDLER_CLASS_NAME);
-            b.invokevirtual(COMBINED_INTERCEPTOR_AND_DECORATOR_STACK_METHOD_HANDLER_CLASS_NAME, "isDisabledHandler", "()" + DescriptorUtils.BOOLEAN_CLASS_DESCRIPTOR);
+
+            // get the Stack
+            b.invokestatic(InterceptionDecorationContext.class.getName(), "getStack", "()" + DescriptorUtils.classToStringRepresentation(Stack.class));
+            b.dupX1(); // Handler, Stack -> Stack, Handler, Stack
+            b.invokevirtual(COMBINED_INTERCEPTOR_AND_DECORATOR_STACK_METHOD_HANDLER_CLASS_NAME, "isDisabledHandler", "(" + DescriptorUtils.classToStringRepresentation(Stack.class) + ")" + DescriptorUtils.BOOLEAN_CLASS_DESCRIPTOR);
+
             b.iconst(0);
             BranchEnd invokeSuperDirectly = b.ifIcmpeq();
             // now build the bytecode that invokes the super class method
+            b.pop2(); // pop Stack and Handler
             b.aload(0);
             // create the method invocation
             b.loadMethodParameters();
             b.invokespecial(methodInfo.getDeclaringClass(), methodInfo.getName(), methodInfo.getDescriptor());
             b.returnInstruction();
             b.branchEnd(invokeSuperDirectly);
+        } else {
+            b.aconstNull();
         }
+
         b.aload(0);
         bytecodeMethodResolver.getDeclaredMethod(method, methodInfo.getDeclaringClass(), methodInfo.getName(), methodInfo.getParameterTypes(), staticConstructor);
 
@@ -318,7 +332,7 @@ protected static void invokeMethodHandler(ClassMethod method, MethodInformation
         }
         // now we have all our arguments on the stack
         // lets invoke the method
-        b.invokeinterface(MethodHandler.class.getName(), "invoke", "(" + LJAVA_LANG_OBJECT + LJAVA_LANG_REFLECT_METHOD + LJAVA_LANG_REFLECT_METHOD + "[" + LJAVA_LANG_OBJECT + ")" + LJAVA_LANG_OBJECT);
+        b.invokeinterface(StackAwareMethodHandler.class.getName(), "invoke", LJAVA_LANG_OBJECT, INVOKE_METHOD_PARAMETERS);
         if (addReturnInstruction) {
             // now we need to return the appropriate type
             if (methodInfo.getReturnType().equals(DescriptorUtils.VOID_CLASS_DESCRIPTOR)) {

impl/src/main/java/org/jboss/weld/bean/proxy/InterceptionDecorationContext.java

@@ -22,6 +22,7 @@
 import java.util.EmptyStackException;
 
 import org.jboss.weld.context.cache.RequestScopedCache;
+import org.jboss.weld.context.cache.RequestScopedItem;
 
 /**
  * A class that holds the interception (and decoration) contexts which are currently in progress.
@@ -39,11 +40,14 @@
 public class InterceptionDecorationContext {
     private static ThreadLocal<Stack> interceptionContexts = new ThreadLocal<Stack>();
 
-    private static class Stack {
+    public static class Stack implements RequestScopedItem {
         private final boolean removeWhenEmpty;
         private final Deque<CombinedInterceptorAndDecoratorStackMethodHandler> elements;
+        private final ThreadLocal<Stack> interceptionContexts;
+        private boolean valid;
 
-        private Stack() {
+        private Stack(ThreadLocal<Stack> interceptionContexts) {
+            this.interceptionContexts = interceptionContexts;
             this.elements = new ArrayDeque<CombinedInterceptorAndDecoratorStackMethodHandler>();
             /*
              * Setting / removing of a thread-local is much more expensive compared to get. Therefore,
@@ -52,12 +56,63 @@ private Stack() {
              * If it is not, the performance characteristics are similar to explicitly removing the thread-local
              * once the stack gets empty.
              */
-            this.removeWhenEmpty = !RequestScopedCache.addItemIfActive(interceptionContexts);
+            this.removeWhenEmpty = !RequestScopedCache.addItemIfActive(this);
+            this.valid = true;
         }
 
         private boolean shouldRemove() {
             return removeWhenEmpty && elements.isEmpty();
         }
+
+        /**
+         * Pushes the given context to the stack if the given context is not on top of the stack already.
+         * If push happens, the caller is responsible for calling {@link #endInterceptorContext()} after the invocation finishes.
+         * @param context the given context
+         * @return true if the given context was pushed to the top of the stack, false if the given context was on top already
+         */
+        public boolean startIfNotOnTop(CombinedInterceptorAndDecoratorStackMethodHandler context) {
+            checkState();
+            if (elements.isEmpty() || peek() != context) {
+                push(context);
+                return true;
+            }
+            return false;
+        }
+
+        public void end() {
+            pop();
+        }
+
+        private void push(CombinedInterceptorAndDecoratorStackMethodHandler item) {
+            checkState();
+            elements.addFirst(item);
+        }
+
+        public CombinedInterceptorAndDecoratorStackMethodHandler peek() {
+            checkState();
+            return elements.peekFirst();
+        }
+
+        private CombinedInterceptorAndDecoratorStackMethodHandler pop() {
+            checkState();
+            CombinedInterceptorAndDecoratorStackMethodHandler top = elements.removeFirst();
+            if (shouldRemove()) {
+                invalidate();
+            }
+            return top;
+        }
+
+        private void checkState() {
+            if (!valid) {
+                throw new IllegalStateException("This InterceptionDecorationContext is no longer valid.");
+            }
+        }
+
+        @Override
+        public void invalidate() {
+            interceptionContexts.remove();
+            valid = false;
+        }
     }
 
     private InterceptionDecorationContext() {
@@ -78,10 +133,10 @@ public static CombinedInterceptorAndDecoratorStackMethodHandler peek() {
      */
     public static CombinedInterceptorAndDecoratorStackMethodHandler peekIfNotEmpty() {
         Stack stack = interceptionContexts.get();
-        if (empty(stack)) {
+        if (stack == null) {
             return null;
         }
-        return peek(stack);
+        return stack.peek();
     }
 
     /**
@@ -109,7 +164,7 @@ public static boolean startIfNotEmpty() {
         if (empty(stack)) {
             return false;
         }
-        push(interceptionContexts.get(), CombinedInterceptorAndDecoratorStackMethodHandler.NULL_INSTANCE);
+        stack.push(CombinedInterceptorAndDecoratorStackMethodHandler.NULL_INSTANCE);
         return true;
     }
 
@@ -120,41 +175,35 @@ public static boolean startIfNotEmpty() {
      * @return true if the given context was pushed to the top of the stack, false if the given context was on top already
      */
     public static boolean startIfNotOnTop(CombinedInterceptorAndDecoratorStackMethodHandler context) {
+        return getStack().startIfNotOnTop(context);
+    }
+
+    /**
+     * Gets the current Stack. If the stack is not set, a new empty instance is created and set.
+     * @return
+     */
+    public static Stack getStack() {
         Stack stack = interceptionContexts.get();
-        if (empty(stack) || peek(stack) != context) { // == used intentionally instead of equals
-            push(stack, context);
-            return true;
+        if (stack == null) {
+            stack = new Stack(interceptionContexts);
+            interceptionContexts.set(stack);
         }
-        return false;
+        return stack;
     }
 
     private static CombinedInterceptorAndDecoratorStackMethodHandler pop(Stack stack) {
         if (stack == null) {
             throw new EmptyStackException();
         } else {
-            try {
-                return stack.elements.removeFirst();
-            } finally {
-                if (stack.shouldRemove()) {
-                    interceptionContexts.remove();
-                }
-            }
-        }
-    }
-
-    private static void push(Stack stack, CombinedInterceptorAndDecoratorStackMethodHandler item) {
-        if (stack == null) {
-            stack = new Stack();
-            interceptionContexts.set(stack);
+            return stack.pop();
         }
-        stack.elements.addFirst(item);
     }
 
     private static CombinedInterceptorAndDecoratorStackMethodHandler peek(Stack stack) {
         if (stack == null) {
             throw new EmptyStackException();
         } else {
-            return stack.elements.peekFirst();
+            return stack.peek();
         }
     }
 

impl/src/main/java/org/jboss/weld/bean/proxy/StackAwareMethodHandler.java

@@ -0,0 +1,45 @@
+/*
+ * JBoss, Home of Professional Open Source
+ * Copyright 2014, Red Hat, Inc., and individual contributors
+ * by the @authors tag. See the copyright.txt in the distribution for a
+ * full listing of individual contributors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.jboss.weld.bean.proxy;
+
+import java.lang.reflect.Method;
+
+import org.jboss.weld.bean.proxy.InterceptionDecorationContext.Stack;
+
+/**
+ * The interface implemented by the invocation handler of a proxy instance. Implementations of this interface get the current
+ * {@link InterceptionDecorationContext} stack passed as a parameter. That way, the number the ThreadLocal access is reduced.
+ *
+ * @author Jozef Hartinger
+ */
+public interface StackAwareMethodHandler extends MethodHandler {
+
+    /**
+     * Is called when a method is invoked on a proxy instance associated with this handler. This method must process that method invocation.
+     *
+     * @param the current {@link InterceptionDecorationContext} stack
+     * @param self the proxy instance.
+     * @param thisMethod the overridden method declared in the super class or interface.
+     * @param proceed the forwarder method for invoking the overridden method. It is null if the overridden method is abstract or declared in the interface.
+     * @param args an array of objects containing the values of the arguments passed in the method invocation on the proxy instance. If a parameter type is a
+     *        primitive type, the type of the array element is a wrapper class.
+     * @return the resulting value of the method invocation.
+     *
+     * @throws Throwable if the method invocation fails.
+     */
+    Object invoke(Stack stack, Object self, Method thisMethod, Method proceed, Object[] args) throws Throwable;
+}