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