Hi Adam,
This issue should be resolved once I remerge Jacob's
EL implementation back into JSF_1_1_ROLLING.
Adam Winer wrote:
> Just got the latest numbers for these fixes, which show
> up to a 4% further improvement in scalability (this not
> including the significant improvement already from
> resolving issue 203).
>
> We're now down to only one significant sync wait in the
> JSF RI codebase: the SynchronizedMap.get() call in
> ExpressionEvaluatorImpl.parseExpressionString(). This
> is less serious than the other sync waits, and also does
> not have a transparently trivial fix (since eliminating
> the cache would likely significantly hurt performance).
>
> However, one thought would be if a ValueBindingImpl object
> could cache the parsed expression after the first call to
> getValue(), setValue(), getType(), or isReadOnly();
> many expressions get evaluated over and over and over and
> over again; in particular, expressions in a table, which
> happens to be what we're testing here. I've no idea whether
> this is practical.
>
> Thoughts?
>
> -- Adam
>
>
>
>
> Ryan Lubke wrote:
>
>> Changes for issues 205 and 206 (JSF_1_1_ROLLING)
>>
>>
>> SECTION: Modified Files
>> ----------------------------
>> M build-tests.xml
>> - removed InstancePool test (Issue 206)
>>
>> M src/com/sun/faces/application/ApplicationAssociate.java
>> - removed related items to InstancePool (Issue 206)
>>
>> M src/com/sun/faces/application/ApplicationImpl.java
>> - pass application to MixedELValueBinding ctor (Issue 206)
>>
>> M src/com/sun/faces/el/MixedELValueBinding.java
>> M src/com/sun/faces/el/ValueBindingImpl.java
>> - Originally these implementations created a new
>> ExpressionInfo for each get/setValue call.
>> Eventaully, to reduce the number of objects,
>> InstancePool was added.
>> Removed InstancePool and created a 1:1 ration between
>> ValueBinding and ExpressionInfo (Issue 206)
>>
>> M src/com/sun/faces/lifecycle/LifecycleImpl.java
>> - Port if issue 66 (Issue 205)
>>
>> R src/com/sun/faces/util/InstancePool.java
>> R src/com/sun/faces/util/JSFBitSet.java
>> R test/com/sun/faces/util/TestInstancePool_local.java
>> - removed (Issue 206)
>>
>>
>> SECTION: Diffs
>> ----------------------------
>> Index: build-tests.xml
>> ===================================================================
>> RCS file: /cvs/javaserverfaces-sources/jsf-ri/build-tests.xml,v
>> retrieving revision 1.204.28.3
>> diff -u -r1.204.28.3 build-tests.xml
>> --- build-tests.xml 1 Dec 2005 16:43:46 -0000 1.204.28.3
>> +++ build-tests.xml 2 Dec 2005 23:13:36 -0000
>> @@ -392,7 +392,6 @@
>> <formatter type="xml" usefile="true"/>
>> <jvmarg
>> line="-Djcov.file=${out.test.dir}/jsf-impl-junit.jcov"/>
>> <test todir="${test.results.dir}"
>> name="com.sun.faces.util.TestUtil_local"/>
>> - <test todir="${test.results.dir}"
>> name="com.sun.faces.util.TestInstancePool_local"/>
>> </junit>
>>
>> </target>
>> Index: src/com/sun/faces/application/ApplicationAssociate.java
>> ===================================================================
>> RCS file:
>> /cvs/javaserverfaces-sources/jsf-ri/src/com/sun/faces/application/ApplicationAssociate.java,v
>>
>> retrieving revision 1.3.26.1.2.1
>> diff -u -r1.3.26.1.2.1 ApplicationAssociate.java
>> --- src/com/sun/faces/application/ApplicationAssociate.java 7 Nov
>> 2005 22:04:37 -0000 1.3.26.1.2.1
>> +++ src/com/sun/faces/application/ApplicationAssociate.java 2 Dec
>> 2005 23:13:36 -0000
>> @@ -10,7 +10,6 @@
>> package com.sun.faces.application;
>>
>> import com.sun.faces.util.Util;
>> -import com.sun.faces.util.InstancePool;
>> import com.sun.faces.RIConstants;
>> import com.sun.faces.config.ConfigureListener;
>>
>> @@ -42,7 +41,7 @@
>> * handle the rest.</p>
>> */
>>
>> -public class ApplicationAssociate extends Object {
>> +public class ApplicationAssociate {
>>
>> protected static Log log = LogFactory.getLog(ApplicationImpl.class);
>>
>> @@ -82,14 +81,7 @@
>> private boolean responseRendered = false;
>>
>> private static final String ASSOCIATE_KEY =
>> RIConstants.FACES_PREFIX +
>> - "ApplicationAssociate";
>> -
>> -
>> - /**
>> - * <p>Used by the EL system to pool instances of ExpressionInfo.
>> - * </p>
>> - */
>> - private InstancePool expressionInfoInstancePool;
>> + "ApplicationAssociate"; public ApplicationAssociate() {
>> @@ -107,8 +99,7 @@
>> managedBeanFactoriesMap = new HashMap();
>> caseListMap = new HashMap();
>> wildcardMatchList = new TreeSet(new SortIt());
>> - - expressionInfoInstancePool = new InstancePool();
>> +
>> }
>> public static ApplicationAssociate getInstance(ExternalContext
>> @@ -343,19 +334,6 @@
>>
>> String viewId;
>> ConfigNavigationCase navCase;
>> - }
>> -
>> - /**
>> - * <p>@return the {_at_link InstancePool} that is allocated for the
>> - * purposes of holding {_at_link com.sun.faces.el.impl.ExpressionInfo}
>> - * instances. The ApplicationAssociate does nothing but serve as
>> - * the owning reference of the <code>InstancePool</code>. Actual
>> - * usage of the <code>InstancePool</code> happens in the EL
>> - * package.</p>
>> - */
>> -
>> - public InstancePool getExpressionInfoInstancePool() {
>> - return expressionInfoInstancePool;
>> }
>>
>> }
>> Index: src/com/sun/faces/application/ApplicationImpl.java
>> ===================================================================
>> RCS file:
>> /cvs/javaserverfaces-sources/jsf-ri/src/com/sun/faces/application/ApplicationImpl.java,v
>>
>> retrieving revision 1.52.28.2
>> diff -u -r1.52.28.2 ApplicationImpl.java
>> --- src/com/sun/faces/application/ApplicationImpl.java 8 Nov 2005
>> 02:36:03 -0000 1.52.28.2
>> +++ src/com/sun/faces/application/ApplicationImpl.java 2 Dec 2005
>> 23:13:36 -0000
>> @@ -283,7 +283,7 @@
>>
>> // is this a Mixed expression?
>> if (Util.isMixedVBExpression(ref)) {
>> - valueBinding = new MixedELValueBinding();
>> + valueBinding = new MixedELValueBinding(this);
>> } else {
>> // PENDING: Need to impelement the performance
>> enhancement
>> // suggested by Hans in the EG on 17 November 2003.
>> Index: src/com/sun/faces/el/MixedELValueBinding.java
>> ===================================================================
>> RCS file:
>> /cvs/javaserverfaces-sources/jsf-ri/src/com/sun/faces/el/Attic/MixedELValueBinding.java,v
>>
>> retrieving revision 1.5
>> diff -u -r1.5 MixedELValueBinding.java
>> --- src/com/sun/faces/el/MixedELValueBinding.java 31 Mar 2004
>> 18:48:29 -0000 1.5
>> +++ src/com/sun/faces/el/MixedELValueBinding.java 2 Dec 2005
>> 23:13:36 -0000
>> @@ -31,8 +31,15 @@
>> private static final Log log =
>> LogFactory.getLog(MixedELValueBinding.class);
>>
>> - private String ref = null;
>> + private String ref = null; private List expressions = null;
>> + + public MixedELValueBinding() { }
>> +
>> + public MixedELValueBinding(Application application) {
>> + super(application);
>> + exprInfo.setExpectedType(String.class); + }
>>
>>
>> public void setRef(String ref) {
>> @@ -66,18 +73,13 @@
>> }
>>
>> StringBuffer sb = new StringBuffer();
>> - Iterator i = expressions.iterator();
>> - Application application = context.getApplication();
>> - ExpressionInfo info = new ExpressionInfo();
>> - info.setExpectedType(String.class);
>> - info.setFacesContext(context);
>> - info.setVariableResolver(application.getVariableResolver());
>> - info.setPropertyResolver(application.getPropertyResolver());
>> + Iterator i = expressions.iterator();
>> + exprInfo.setFacesContext(context); while
>> (i.hasNext()) {
>> Object o = i.next();
>> if (o instanceof Expression) {
>> try {
>> - Object value = ((Expression) o).evaluate(info);
>> + Object value = ((Expression) o).evaluate(exprInfo);
>> if (value != null) {
>> sb.append(value);
>> } // null values are effectively zero length strings
>> @@ -91,7 +93,7 @@
>> log.debug("getValue Evaluation threw
>> exception:", t);
>> }
>> throw new EvaluationException(t);
>> - }
>> + }
>> } else {
>> sb.append(o);
>> }
>> @@ -138,6 +140,10 @@
>>
>> public void restoreState(FacesContext context, Object state) {
>> ref = state.toString();
>> + Application application = context.getApplication();
>> +
>> exprInfo.setPropertyResolver(application.getPropertyResolver());
>> +
>> exprInfo.setVariableResolver(application.getVariableResolver());
>> + exprInfo.setExpectedType(String.class);
>> }
>>
>>
>> Index: src/com/sun/faces/el/ValueBindingImpl.java
>> ===================================================================
>> RCS file:
>> /cvs/javaserverfaces-sources/jsf-ri/src/com/sun/faces/el/Attic/ValueBindingImpl.java,v
>>
>> retrieving revision 1.35
>> diff -u -r1.35 ValueBindingImpl.java
>> --- src/com/sun/faces/el/ValueBindingImpl.java 17 Jul 2004
>> 01:37:12 -0000 1.35
>> +++ src/com/sun/faces/el/ValueBindingImpl.java 2 Dec 2005 23:13:36
>> -0000
>> @@ -9,20 +9,8 @@
>>
>> package com.sun.faces.el;
>>
>> -import com.sun.faces.RIConstants;
>> -import com.sun.faces.el.impl.ElException;
>> -import com.sun.faces.el.impl.Expression;
>> -import com.sun.faces.el.impl.ExpressionInfo;
>> -import com.sun.faces.util.Util;
>> -import com.sun.faces.util.InstancePool;
>> -import com.sun.faces.util.InstancePool.NewInstance;
>> -import com.sun.faces.application.ApplicationAssociate;
>> -import org.apache.commons.logging.Log;
>> -import org.apache.commons.logging.LogFactory;
>> -
>> import javax.faces.application.Application;
>> import javax.faces.component.StateHolder;
>> -import javax.faces.context.ExternalContext;
>> import javax.faces.context.FacesContext;
>> import javax.faces.el.EvaluationException;
>> import javax.faces.el.PropertyNotFoundException;
>> @@ -30,7 +18,14 @@
>> import javax.faces.el.ValueBinding;
>>
>> import java.util.Arrays;
>> -import java.util.Map;
>> +
>> +import org.apache.commons.logging.Log;
>> +import org.apache.commons.logging.LogFactory;
>> +
>> +import com.sun.faces.el.impl.ElException;
>> +import com.sun.faces.el.impl.Expression;
>> +import com.sun.faces.el.impl.ExpressionInfo;
>> +import com.sun.faces.util.Util;
>>
>> public class ValueBindingImpl extends ValueBinding implements
>> StateHolder {
>>
>> @@ -72,13 +67,10 @@
>>
>> // Relationship Instance Variables
>>
>> - protected String ref = null;
>> -
>> - protected Application application = null;
>> -
>> - protected ApplicationAssociate appAssociate = null;
>> -
>> - protected static Map applicationMap = null;
>> + protected String ref;
>> + protected String exprString;
>> + protected ExpressionInfo exprInfo = new
>> ExpressionInfo(); + //
>> // Constructors and Initializers @@ -92,29 +84,10 @@
>>
>> public ValueBindingImpl(Application application) {
>> Util.parameterNonNull(application);
>> - //PENDING(rogerk)getCurrentinstance() performance considerations.
>> - FacesContext facesContext = FacesContext.getCurrentInstance();
>> - this.application = application;
>> - this.appAssociate =
>> -
>> ApplicationAssociate.getInstance(facesContext.getExternalContext());
>> - - if (null == applicationMap) {
>> - applicationMap =
>> - facesContext.getExternalContext()
>> - .getApplicationMap();
>> - }
>> -
>> - if (null != this.appAssociate) {
>> - InstancePool pool =
>> this.appAssociate.getExpressionInfoInstancePool();
>> - if (!pool.isInitialized()) {
>> - pool.setInstantiator(new NewInstance() {
>> - public Object newInstance() {
>> - return new ExpressionInfo();
>> - }
>> - });
>> - }
>> - } - Util.doAssert(null !=
>> applicationMap);
>> + +
>> exprInfo.setVariableResolver(application.getVariableResolver());
>> +
>> exprInfo.setPropertyResolver(application.getPropertyResolver());
>> + }
>>
>> //
>> @@ -129,6 +102,7 @@
>> reset();
>> Util.parameterNonEmpty(newRef);
>> ref = newRef;
>> + exprString = getExprString(ref);
>> }
>>
>>
>> @@ -146,7 +120,7 @@
>> throw new NullPointerException(
>>
>> Util.getExceptionMessageString(Util.NULL_CONTEXT_ERROR_MESSAGE_ID));
>> }
>> - Object result = null;
>> + Object result;
>>
>> if (log.isDebugEnabled()) {
>> log.debug("getValue(ref=" + ref + ")");
>> @@ -161,16 +135,14 @@
>>
>> protected Object getValue(FacesContext context, String toEvaluate)
>> throws EvaluationException, PropertyNotFoundException {
>> - Object result = null;
>> + Object result;
>> +
>>
>> - ExpressionInfo info = checkoutExpressionInfo();
>> try {
>> - info.setExpressionString(toEvaluate);
>> - info.setExpectedType(Object.class);
>> - info.setFacesContext(context);
>> -
>> info.setVariableResolver(application.getVariableResolver());
>> -
>> info.setPropertyResolver(application.getPropertyResolver());
>> - result = Util.getExpressionEvaluator().evaluate(info);
>> + exprInfo.setExpressionString(toEvaluate);
>> + exprInfo.setExpectedType(Object.class);
>> + exprInfo.setFacesContext(context);
>> + result = Util.getExpressionEvaluator().evaluate(exprInfo);
>> if (log.isDebugEnabled()) {
>> log.debug("getValue Result:" + result);
>> }
>> @@ -183,8 +155,7 @@
>> l = t;
>> }
>> log.debug("getValue Evaluation threw exception:",
>> l);
>> - }
>> - checkinExpressionInfo(info);
>> + } throw new
>> EvaluationException(e);
>> } else if (e instanceof PropertyNotFoundException) {
>> if (log.isDebugEnabled()) {
>> @@ -195,18 +166,16 @@
>> }
>> log.debug("getValue Evaluation threw exception:",
>> l);
>> }
>> - // Just rethrow it to keep detailed message
>> - checkinExpressionInfo(info);
>> + // Just rethrow it to keep detailed
>> message throw
>> (PropertyNotFoundException) e;
>> } else {
>> if (log.isDebugEnabled()) {
>> log.debug("getValue Evaluation threw exception:",
>> e);
>> - }
>> - checkinExpressionInfo(info);
>> + } throw new
>> EvaluationException(e);
>> }
>> - }
>> - checkinExpressionInfo(info);
>> + }
>> + return result;
>> }
>>
>> @@ -223,17 +192,13 @@
>> Util.ILLEGAL_IDENTIFIER_LVALUE_MODE_ID, new
>> Object[]{ref}));
>> }
>> // PENDING(edburns): check for readOnly-ness -
>> ExpressionInfo info = checkoutExpressionInfo();
>> +
>> try {
>> - info.setExpressionString(ref);
>> - info.setFacesContext(context);
>> -
>> info.setVariableResolver(application.getVariableResolver());
>> -
>> info.setPropertyResolver(application.getPropertyResolver());
>> + exprInfo.setExpressionString(ref);
>> +
>> exprInfo.setFacesContext(context); Expression
>> expr =
>> - Util.getExpressionEvaluator().parseExpression(info);
>> - expr.setValue(info, value);
>> - checkinExpressionInfo(info);
>> - return;
>> +
>> Util.getExpressionEvaluator().parseExpression(exprInfo);
>> + expr.setValue(exprInfo, value);
>> } catch (Throwable e) {
>> if (e instanceof ElException) {
>> if (log.isDebugEnabled()) {
>> @@ -243,8 +208,7 @@
>> l = t;
>> }
>> log.debug("setValue Evaluation threw exception:",
>> l);
>> - }
>> - checkinExpressionInfo(info);
>> + } throw new
>> EvaluationException(e);
>> } else if (e instanceof PropertyNotFoundException) {
>> if (log.isDebugEnabled()) {
>> @@ -255,24 +219,20 @@
>> }
>> log.debug("setValue Evaluation threw exception:",
>> l);
>> }
>> - // Just rethrow it to keep detailed message
>> - checkinExpressionInfo(info);
>> + // Just rethrow it to keep detailed
>> message throw
>> (PropertyNotFoundException) e;
>> } else if (e instanceof EvaluationException) {
>> if (log.isDebugEnabled()) {
>> log.debug("setValue Evaluation threw exception:",
>> e);
>> - }
>> - checkinExpressionInfo(info);
>> - throw ((EvaluationException)e);
>> - }
>> - else {
>> + } + throw
>> ((EvaluationException) e);
>> + } else {
>> if (log.isDebugEnabled()) {
>> log.debug("setValue Evaluation threw exception:",
>> e);
>> - }
>> - checkinExpressionInfo(info);
>> + } throw new
>> EvaluationException(e);
>> }
>> - }
>> + }
>> }
>>
>>
>> @@ -282,17 +242,13 @@
>> throw new NullPointerException(
>>
>> Util.getExceptionMessageString(Util.NULL_CONTEXT_ERROR_MESSAGE_ID));
>> }
>> - ExpressionInfo info = checkoutExpressionInfo();
>> +
>> try {
>> - info.setExpressionString(ref);
>> - info.setFacesContext(context);
>> -
>> info.setVariableResolver(application.getVariableResolver());
>> -
>> info.setPropertyResolver(application.getPropertyResolver());
>> + exprInfo.setExpressionString(ref);
>> +
>> exprInfo.setFacesContext(context); Expression
>> expr =
>> - Util.getExpressionEvaluator().parseExpression(info);
>> - boolean result = expr.isReadOnly(info);
>> - checkinExpressionInfo(info);
>> - return result;
>> +
>> Util.getExpressionEvaluator().parseExpression(exprInfo);
>> + return expr.isReadOnly(exprInfo); }
>> catch (Throwable e) {
>> if (e instanceof ElException) {
>> if (log.isDebugEnabled()) {
>> @@ -303,7 +259,6 @@
>> }
>> log.debug("isReadOnly Evaluation threw
>> exception:", l);
>> }
>> - checkinExpressionInfo(info);
>> throw new EvaluationException(e);
>> } else if (e instanceof PropertyNotFoundException) {
>> if (log.isDebugEnabled()) {
>> @@ -315,16 +270,14 @@
>> log.debug("isReadOnly Evaluation threw
>> exception:", l);
>> }
>> // Just rethrow it to keep detailed message
>> - checkinExpressionInfo(info);
>> throw (PropertyNotFoundException) e;
>> } else {
>> if (log.isDebugEnabled()) {
>> log.debug("isReadOnly Evaluation threw
>> exception:", e);
>> }
>> - checkinExpressionInfo(info);
>> throw new EvaluationException(e);
>> }
>> - }
>> + }
>> }
>>
>>
>> @@ -334,17 +287,13 @@
>> throw new NullPointerException(
>>
>> Util.getExceptionMessageString(Util.NULL_CONTEXT_ERROR_MESSAGE_ID));
>> }
>> - ExpressionInfo info = checkoutExpressionInfo();
>> +
>> try {
>> - info.setExpressionString(ref);
>> - info.setFacesContext(context);
>> -
>> info.setVariableResolver(application.getVariableResolver());
>> -
>> info.setPropertyResolver(application.getPropertyResolver());
>> + exprInfo.setExpressionString(ref);
>> +
>> exprInfo.setFacesContext(context); Expression
>> expr =
>> - Util.getExpressionEvaluator().parseExpression(info);
>> - Class result = expr.getType(info);
>> - checkinExpressionInfo(info);
>> - return result;
>> +
>> Util.getExpressionEvaluator().parseExpression(exprInfo);
>> + return expr.getType(exprInfo); }
>> catch (Throwable e) {
>> if (e instanceof ElException) {
>> if (log.isDebugEnabled()) {
>> @@ -355,7 +304,6 @@
>> }
>> log.debug("getType Evaluation threw exception:", l);
>> }
>> - checkinExpressionInfo(info);
>> throw new EvaluationException(e);
>> } else if (e instanceof PropertyNotFoundException) {
>> if (log.isDebugEnabled()) {
>> @@ -367,21 +315,19 @@
>> log.debug("getType Evaluation threw exception:", l);
>> }
>> // Just rethrow it to keep detailed message
>> - checkinExpressionInfo(info);
>> throw (PropertyNotFoundException) e;
>> } else {
>> if (log.isDebugEnabled()) {
>> log.debug("getType Evaluation threw exception:", e);
>> }
>> - checkinExpressionInfo(info);
>> throw new EvaluationException(e);
>> }
>> - }
>> + }
>> }
>>
>>
>> public String getExpressionString() {
>> - return "#{" + ref + "}";
>> + return exprString;
>> }
>>
>> /**
>> @@ -401,19 +347,16 @@
>> //
>>
>> public Object saveState(FacesContext context) {
>> - Object result = ref;
>> - return result;
>> + return ref;
>> }
>>
>>
>> public void restoreState(FacesContext context, Object state) {
>> ref = state.toString();
>> - if (null == application) {
>> - application = context.getApplication();
>> - }
>> - if (null == applicationMap) {
>> - applicationMap =
>> context.getExternalContext().getApplicationMap();
>> - }
>> + exprString = getExprString(ref);
>> + Application application = context.getApplication();
>> +
>> exprInfo.setPropertyResolver(application.getPropertyResolver());
>> +
>> exprInfo.setVariableResolver(application.getVariableResolver());
>> }
>>
>>
>> @@ -430,26 +373,9 @@
>> }
>>
>> // helper methods
>> -
>> - private ExpressionInfo checkoutExpressionInfo() {
>> - ExpressionInfo result = null;
>> - if (null != appAssociate) {
>> - // PENDING(edburns): generic types would be nice here
>> - result = (ExpressionInfo)
>> - appAssociate.getExpressionInfoInstancePool().checkout();
>> - result.reset();
>> - }
>> - else {
>> - result = new ExpressionInfo();
>> - }
>> - return result;
>> - }
>> - private void checkinExpressionInfo(ExpressionInfo toCheckin) {
>> - if (null != appAssociate) {
>> -
>> appAssociate.getExpressionInfoInstancePool().checkin(toCheckin);
>> - }
>> + private static String getExprString(String ref) {
>> + return "#{" + ref + '}';
>> }
>> -
>>
>> } // end of class ValueBindingImpl
>> Index: src/com/sun/faces/lifecycle/LifecycleImpl.java
>> ===================================================================
>> RCS file:
>> /cvs/javaserverfaces-sources/jsf-ri/src/com/sun/faces/lifecycle/LifecycleImpl.java,v
>>
>> retrieving revision 1.45.28.2
>> diff -u -r1.45.28.2 LifecycleImpl.java
>> --- src/com/sun/faces/lifecycle/LifecycleImpl.java 3 Feb 2005
>> 20:08:36 -0000 1.45.28.2
>> +++ src/com/sun/faces/lifecycle/LifecycleImpl.java 2 Dec 2005
>> 23:13:36 -0000
>> @@ -164,7 +164,9 @@
>> + "," + listener);
>> }
>> synchronized (listeners) {
>> - listeners.remove(listener);
>> + ArrayList temp = (ArrayList) listeners.clone();
>> + temp.remove(listener);
>> + listeners = temp;
>> }
>>
>> }
>> @@ -181,73 +183,72 @@
>> log.trace("phase(" + phaseId.toString() + "," + context +
>> ")");
>> }
>>
>> - int
>> - i = 0,
>> - maxBefore = 0;
>> - List tempListeners = (ArrayList)listeners.clone();
>> - try {
>> - // Notify the "beforePhase" method of interested listeners
>> - // (ascending)
>> - // Fix for bug 6223295.Clone the'listeners' list so that
>> - // we don't see any mofifications made to it during the
>> execution of
>> - // this method. So any listener added would not show up
>> - // until the NEXT phase but we need to live this to
>> avoid the lengthy
>> - // synchronization block. Due to this, "listeners"
>> should be
>> - // modified only via add/remove methods and must never
>> be updated
>> - // directly.
>> - if (tempListeners.size() > 0) {
>> - PhaseEvent event = new PhaseEvent(context, phaseId,
>> this);
>> - for (i = 0; i < tempListeners.size(); i++) {
>> - PhaseListener listener =
>> (PhaseListener)tempListeners.get(i);
>> - if (phaseId.equals(listener.getPhaseId()) ||
>> -
>> PhaseId.ANY_PHASE.equals(listener.getPhaseId())) {
>> + // grab a pointer to our listeners
>> + List ourListeners = this.listeners;
>> + int numListeners = ourListeners.size();
>> + + // build a list of all listeners fired
>> + List firedListeners = new ArrayList(numListeners);
>> + + // if we have PhaseListeners to fire
>> + if (numListeners > 0) {
>> + // instantiate all of our variables
>> + PhaseEvent event = new PhaseEvent(context, phaseId, this);
>> + PhaseListener listener;
>> + PhaseId listenerPhaseId;
>> + int i = 0;
>> + + // call all relevant PhaseListeners before
>> + try {
>> + while (i < numListeners) {
>> + listener = (PhaseListener) ourListeners.get(i);
>> + listenerPhaseId = listener.getPhaseId();
>> + if (phaseId.equals(listenerPhaseId)
>> + || PhaseId.ANY_PHASE.equals(listenerPhaseId)) {
>> listener.beforePhase(event);
>> + firedListeners.add(listener);
>> }
>> - maxBefore = i;
>> + i++;
>> }
>> }
>> - }
>> - catch (Throwable e) {
>> - if (log.isTraceEnabled()) {
>> - log.trace("phase(" + phaseId.toString() + "," + context +
>> - ") threw exception: " + e + " " + e.getMessage() +
>> - "\n" + Util.getStackTraceString(e));
>> - }
>> - }
>> - - try { - // Execute this phase itself (if still
>> needed)
>> - if (!skipping(phaseId, context)) {
>> - phase.execute(context);
>> - }
>> - }
>> - finally {
>> - try {
>> - // Notify the "afterPhase" method of interested listeners
>> - // (descending)
>> - if (tempListeners.size() > 0) {
>> - PhaseEvent event = new PhaseEvent(context,
>> phaseId, this);
>> - for (i = maxBefore; i >= 0; i--) {
>> - PhaseListener listener = (PhaseListener)
>> - tempListeners.get(i);
>> - if (phaseId.equals(listener.getPhaseId()) ||
>> -
>> PhaseId.ANY_PHASE.equals(listener.getPhaseId())) {
>> - listener.afterPhase(event);
>> - }
>> + catch (Throwable e) {
>> + if (log.isTraceEnabled()) {
>> + log.trace("beforePhase(" + phaseId.toString() +
>> "," + context +
>> + ") threw exception: " + e + " " +
>> e.getMessage() +
>> + "\n" + Util.getStackTraceString(e));
>> + }
>> + }
>> + try
>> + {
>> + // if response is not already handled, execute phase
>> + if (!skipping(phaseId, context)) {
>> + phase.execute(context);
>> + }
>> + }
>> + finally {
>> + // call all relevant PhaseListeners after
>> + i = firedListeners.size() - 1;
>> + try {
>> + while (i >= 0) {
>> + listener = (PhaseListener)
>> firedListeners.get(i);
>> + listener.afterPhase(event);
>> + i--;
>> + }
>> + } catch (Throwable e) {
>> + if (log.isTraceEnabled()) {
>> + log.trace("afterPhase(" + phaseId.toString()
>> + "," + context +
>> + ") threw exception: " + e + " " +
>> e.getMessage() +
>> + "\n" + Util.getStackTraceString(e));
>> }
>> }
>> - }
>> - catch (Throwable e) {
>> - if (log.isTraceEnabled()) {
>> - log.trace("phase(" + phaseId.toString() + "," + context +
>> - ") threw exception: " + e + " " + e.getMessage() +
>> - "\n" + Util.getStackTraceString(e));
>> - }
>> - }
>> - }
>> -
>> + }
>> + } else { // else no PhaseListeners and simply execute the Phase
>> + // if response is not already handled, execute phase
>> + if (!skipping(phaseId, context)) {
>> + phase.execute(context);
>> + }
>> + }
>> }
>> -
>>
>> // Return "true" if this request is a browser reload and we just
>> // completed the Restore View phase
>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe_at_javaserverfaces.dev.java.net
>> For additional commands, e-mail: dev-help_at_javaserverfaces.dev.java.net
>>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_javaserverfaces.dev.java.net
> For additional commands, e-mail: dev-help_at_javaserverfaces.dev.java.net
>