From: Roger Kitain <Roger.Kitain_at_Sun.COM>
Date: Mon, 22 May 2006 12:50:22 -0400

r=rogerk

Ryan Lubke wrote:

> Update to ViewHandler state writing.
> - replace the replaceMarkers method (a series of
> substrings and writes) with
> WriteBehindStringWriter.flushToWriter(Writer).
> Performance testing shows an increase
> of 5 to 7 requests per second (server-side state
> saving) using 100 client threads.
>
>
> SECTION: Modified Files
> ----------------------------
> M src/com/sun/faces/RIConstants.java
> - added constant SAVESTATE_FIELD_DELIMITER which
> now present before and after the
> SAVESTATE_FIELD_MARKER - this is to make
> easier to pick the save state field marker
> within the buffered response
>
> M src/com/sun/faces/application/ViewHandlerImpl.java
> - updated javadoc comments
> - added private class WriteBehindStringWriter
> which has a method called flushToWriter(writer)
> which allows the writing of the response
> and the replacement of the state markers
> without creating a series of Strings
> M src/com/sun/faces/config/WebConfiguration.java
> - added new option com.sun.faces.responseBufferSize
> to tune the size of the backing StringBuilder
> as well as the size of the char[] that is
> used to write the response
>
> M src/com/sun/faces/io/FastStringWriter.java
> - made the backing StringBuilder protected
>
> M web/test/CorrectRenderersResponse
> - update the test to include the field delimiters
>
> SECTION: Diffs
> ----------------------------
> Index: src/com/sun/faces/RIConstants.java
> ===================================================================
> RCS file:
> /cvs/javaserverfaces-sources/jsf-ri/src/com/sun/faces/RIConstants.java,v
> retrieving revision 1.86
> diff -u -r1.86 RIConstants.java
> --- src/com/sun/faces/RIConstants.java 17 May 2006 19:00:43
> -0000 1.86
> +++ src/com/sun/faces/RIConstants.java 22 May 2006 16:23:56 -0000
> @@ -45,8 +45,12 @@
> public final static String HTML_BASIC_RENDER_KIT = FACES_PREFIX +
> RenderKitFactory.HTML_BASIC_RENDER_KIT;
> - public static final String SAVESTATE_FIELD_MARKER = FACES_PREFIX +
> - "saveStateFieldMarker";
> + public static final String SAVESTATE_FIELD_DELIMITER = "~";
> + public static final String SAVESTATE_FIELD_MARKER =
> + SAVESTATE_FIELD_DELIMITER
> + + FACES_PREFIX
> + + "saveStateFieldMarker"
> + + SAVESTATE_FIELD_DELIMITER;
>
> public static final String LOGICAL_VIEW_MAP = FACES_PREFIX +
> "logicalViewMap";
> Index: src/com/sun/faces/application/ViewHandlerImpl.java
> ===================================================================
> RCS file:
> /cvs/javaserverfaces-sources/jsf-ri/src/com/sun/faces/application/ViewHandlerImpl.java,v
>
> retrieving revision 1.74
> diff -u -r1.74 ViewHandlerImpl.java
> --- src/com/sun/faces/application/ViewHandlerImpl.java 17 May 2006
> 19:00:44 -0000 1.74
> +++ src/com/sun/faces/application/ViewHandlerImpl.java 22 May 2006
> 16:23:56 -0000
> @@ -36,7 +36,6 @@
> import javax.faces.FacesException;
> import javax.faces.FactoryFinder;
> import javax.faces.application.StateManager;
> -import javax.faces.application.StateManager.SerializedView;
> import javax.faces.application.ViewHandler;
> import javax.faces.component.UIViewRoot;
> import javax.faces.context.ExternalContext;
> @@ -60,6 +59,8 @@
> import java.util.logging.Logger;
>
> import com.sun.faces.RIConstants;
> +import com.sun.faces.config.WebConfiguration;
> +import com.sun.faces.config.WebConfiguration.WebContextInitParameter;
> import com.sun.faces.io.FastStringWriter;
> import com.sun.faces.util.DebugUtil;
> import com.sun.faces.util.MessageUtils;
> @@ -97,22 +98,7 @@
> * or, if that isn't defined, the value of
> <code>DEFAULT_SUFFIX</code>
> */
> private String contextDefaultSuffix;
> -
> - /**
> - * If <code>true</code>, at least one method exists on
> - * the <code>StateManager</code> implementation that
> - * replaces a deprecated method. This will be used
> - * to determine if the non-deprecated methods should
> - * be called.
> - */
> - private boolean nonDeprecatedMethodExists = false;
> -
> - /**
> - * If <code>true</code>, then we've already performed
> - * the check on the <code>StateManager</code> implementation
> - * for at least one method that replace a deprecated method.
> - */
> - private boolean checkedForNonDeprecated = false;
> + private int bufSize = -1;
>
> public ViewHandlerImpl() {
> if (logger.isLoggable(Level.FINE)) {
> @@ -162,8 +148,27 @@
> renderFactory.getRenderKit(context,
> viewToRender.getRenderKitId());
> ResponseWriter oldWriter =
> context.getResponseWriter();
> - Writer strWriter = new FastStringWriter(2048);
> - ResponseWriter newWriter = null;
> + if (bufSize == -1) {
> + synchronized (this) {
> + if (bufSize == -1) {
> + WebConfiguration webConfig =
> + WebConfiguration
> +
> .getInstance(context.getExternalContext());
> + try {
> + bufSize = Integer
> +
> .parseInt(webConfig.getContextInitParameter(
> +
> WebContextInitParameter.ResponseBufferSize));
> + } catch (NumberFormatException nfe) {
> + bufSize = Integer
> +
> .parseInt(WebContextInitParameter.ResponseBufferSize.getDefaultValue());
> + }
> + }
> + }
> + }
> + + WriteBehindStringWriter strWriter =
> + new WriteBehindStringWriter(context, bufSize);
> + ResponseWriter newWriter;
> if (null != oldWriter) {
> newWriter = oldWriter.cloneWithWriter(strWriter);
> } else {
> @@ -180,7 +185,7 @@
> // replace markers in the body content and write it to
> response.
>
> - ResponseWriter responseWriter = null;
> + ResponseWriter responseWriter;
> if (null != oldWriter) {
> responseWriter =
> oldWriter.cloneWithWriter(response.getWriter());
> } else {
> @@ -188,9 +193,12 @@
> }
> context.setResponseWriter(responseWriter);
> - String bodyContent = strWriter.toString();
> - replaceMarkers(bodyContent, context);
> - + if (logger.isLoggable(Level.FINE)) {
> + logger.fine("Size of response for view '" + viewToRender
> + "':"
> + + strWriter.length());
> + }
> + strWriter.flushToWriter(responseWriter);
> + if (null != oldWriter) {
> context.setResponseWriter(oldWriter);
> }
> @@ -238,6 +246,10 @@
> *
> * <p>Flush the response buffer, and remove the after view content
> * from the request scope.</p>
> + *
> + * @param context the <code>FacesContext</code> for the current
> request
> + * @param viewToRender the view to render
> + * @throws IOException if an error occurs rendering the view to
> the client
> */
>
> private void doRenderView(FacesContext context,
> @@ -382,11 +394,15 @@
> * not 2xx, then return true to indicate the response should be
> * immediately flushed by the caller so that conditions such as 404
> * are properly handled.
> + * @param context the <code>FacesContext</code> for the current
> request
> + * @param viewToExecute the view to build
> * @return <code>true</code> if the response should be immediately
> flushed
> - * to the client, otherwise <code>false</code> + * to
> the client, otherwise <code>false</code>
> + * @throws IOException if an error occurs executing the page
> */
> private boolean executePageToBuildView(FacesContext context,
> - UIViewRoot viewToExecute)
> throws IOException, FacesException {
> + UIViewRoot viewToExecute)
> + throws IOException {
>
> if (null == context) {
> String message = MessageUtils.getExceptionMessageString
> @@ -420,7 +436,7 @@
> String newViewId = requestURI;
> // If we have a valid mapping (meaning we were invoked via the
> // FacesServlet) and we're extension mapped, do the replacement.
> - if (mapping != null && !isPrefixMapped(mapping)) {
> + if (!isPrefixMapped(mapping)) {
> if (logger.isLoggable(Level.FINE)) {
> logger.fine( "Found URL pattern mapping to FacesServlet "
> + mapping);
> @@ -455,8 +471,10 @@
> Object originalResponse = extContext.getResponse();
>
> // replace the response with our wrapper
> - ViewHandlerResponseWrapper wrapped = null;
> - extContext.setResponse(wrapped = new
> ViewHandlerResponseWrapper((HttpServletResponse)extContext.getResponse()));
>
> + ViewHandlerResponseWrapper wrapped =
> + new ViewHandlerResponseWrapper(
> + (HttpServletResponse)extContext.getResponse());
> + extContext.setResponse(wrapped);
>
> // build the view by executing the page
> extContext.dispatch(newViewId); @@ -531,12 +549,11 @@
> String message = MessageUtils.getExceptionMessageString
> (MessageUtils.NULL_PARAMETERS_ERROR_MESSAGE_ID,
> "context");
> throw new NullPointerException(message);
> - }
> - String result = null;
> + }
> Map<String,String> requestParamMap = context.getExternalContext()
> .getRequestParameterMap();
> - result = requestParamMap.get(
> + String result = requestParamMap.get(
> ResponseStateManager.RENDER_KIT_ID_PARAM);
>
> if (result == null) {
> @@ -550,17 +567,21 @@
>
>
> /**
> - * Attempts to find a matching locale based on <code>perf</code> and
> + * Attempts to find a matching locale based on <code>pref</code> and
> * list of supported locales, using the matching algorithm
> * as described in JSTL 8.3.2.
> + * @param context the <code>FacesContext</code> for the current
> request
> + * @param pref the preferred locale
> + * @return the Locale based on pref and the matching alogritm
> specified
> + * in JSTL 8.3.2
> */
> - protected Locale findMatch(FacesContext context, Locale perf) {
> + protected Locale findMatch(FacesContext context, Locale pref) {
> Locale result = null;
> Iterator<Locale> it =
> context.getApplication().getSupportedLocales();
> while (it.hasNext()) {
> Locale supportedLocale = it.next();
>
> - if (perf.equals(supportedLocale)) {
> + if (pref.equals(supportedLocale)) {
> // exact match
> result = supportedLocale;
> break;
> @@ -570,7 +591,7 @@
> // preferred locale is "en-US", if one of supported
> // locales is "en-UK", even though its language matches
> // that of the preferred locale, we must ignore it.
> - if
> (perf.getLanguage().equals(supportedLocale.getLanguage()) &&
> + if
> (pref.getLanguage().equals(supportedLocale.getLanguage()) &&
> supportedLocale.getCountry().equals("")) {
> result = supportedLocale;
> }
> @@ -580,7 +601,7 @@
> if (null == result) {
> Locale defaultLocale =
> context.getApplication().getDefaultLocale();
> if (defaultLocale != null) {
> - if ( perf.equals(defaultLocale)) {
> + if ( pref.equals(defaultLocale)) {
> // exact match
> result = defaultLocale;
> } else {
> @@ -589,7 +610,7 @@
> // preferred locale is "en-US", if one of supported
> // locales is "en-UK", even though its language
> matches
> // that of the preferred locale, we must ignore it.
> - if
> (perf.getLanguage().equals(defaultLocale.getLanguage()) &&
> + if
> (pref.getLanguage().equals(defaultLocale.getLanguage()) &&
> defaultLocale.getCountry().equals("")) {
> result = defaultLocale;
> }
> @@ -637,8 +658,10 @@
> }
>
> if (viewId.charAt(0) != '/') {
> - String message =
> MessageUtils.getExceptionMessageString(MessageUtils.ILLEGAL_VIEW_ID_ID,
> - new
> Object[]{viewId});
> + String message =
> + MessageUtils.getExceptionMessageString(
> + MessageUtils.ILLEGAL_VIEW_ID_ID,
> + viewId);
> if (logger.isLoggable(Level.SEVERE)) {
> logger.log(Level.SEVERE, "jsf.illegal_view_id_error",
> viewId);
> }
> @@ -708,8 +731,7 @@
> * @throws NullPointerException if <code>context</code> is null
> */
> private String getFacesMapping(FacesContext context) {
> - // PENDING (rlubke) Need to handle the Portlet case
> -
> + if (context == null) {
> String message = MessageUtils.getExceptionMessageString
> (MessageUtils.NULL_PARAMETERS_ERROR_MESSAGE_ID,
> "context");
> @@ -767,6 +789,7 @@
> * @param servletPath the servlet path of the request
> * @param pathInfo the path info of the request
> * @see HttpServletRequest#getServletPath()
> + * @return the appropriate mapping based on the current request
> */
> private String getMappingForRequest(String servletPath, String
> pathInfo) {
>
> @@ -888,62 +911,120 @@
>
> }
> return convertedViewId;
> - }
> -
> - @SuppressWarnings("Deprecation")
> - public void replaceMarkers(String content, FacesContext context) {
> - SerializedView view = null;
> - Object stateArray = null;
> - StateManager stateManager = Util.getStateManager(context);
> - try {
> - if (!checkedForNonDeprecated) {
> - nonDeprecatedMethodExists =
> - Util.hasDeclaredMethod(stateManager, "saveView");
> - checkedForNonDeprecated = true;
> - }
> - if (nonDeprecatedMethodExists) {
> - stateArray = stateManager.saveView(context);
> - } else {
> - view = stateManager.saveSerializedView(context);
> - }
> - } catch (IllegalStateException ise) {
> - throw new FacesException(ise);
> - } catch (Exception ie) {
> - // catch any exception thrown while saving the
> view - throw new
> FacesException(MessageUtils.getExceptionMessageString(
> - MessageUtils.SAVING_STATE_ERROR_MESSAGE_ID), ie);
> - }
> - int
> - beginIndex = 0,
> - markerIndex = 0,
> - markerLen = RIConstants.SAVESTATE_FIELD_MARKER.length(),
> - contentLen = 0;
> -
> - try {
> - contentLen = content.length();
> - do {
> - // if we have no more markers
> - if (-1 == (markerIndex =
> - content.indexOf(RIConstants.SAVESTATE_FIELD_MARKER,
> - beginIndex))) {
> - // write out the rest of the content
> -
> context.getResponseWriter().write(content.substring(beginIndex));
> - } else {
> - // we have more markers, write out the current chunk
> -
> - context.getResponseWriter().write(content.substring(
> - beginIndex, markerIndex));
> - if (nonDeprecatedMethodExists) {
> - stateManager.writeState(context, stateArray);
> + } + + //
> ----------------------------------------------------------- Inner Classes
> +
> + /**
> + * <p>Handles writing the response from the dispatched request
> + * and replaces any state markers with the actual
> + * state supplied by the <code>StateManager</code>.
> + */
> + private static final class WriteBehindStringWriter extends
> FastStringWriter {
> + + // lenght of the state marker
> + private static final int STATE_MARKER_LEN =
> + RIConstants.SAVESTATE_FIELD_MARKER.length();
> + + // the context for the current request
> + private final FacesContext context;
> + + // char buffer
> + private final char[] buf;
> + + // buffer length
> + private final int bufSize; + +
> + /**
> + * <p>Create a new <code>WriteBehindStringWriter</code> for
> the current
> + * request with an initial capacity.</p>
> + * @param context the <code>FacesContext</code> for the
> current request
> + * @param initialCapcity the StringBuilder's initial capacity
> + */
> + public WriteBehindStringWriter(FacesContext context, int
> initialCapcity) {
> + super(initialCapcity); + this.context
> = context; + bufSize = initialCapcity;
> + buf = new char[bufSize];
> + }
> +
> + /**
> + * <p> Write directly from our StringBuilder to the provided
> + * writer.</p>
> + * @param writer where to write
> + * @throws IOException if an error occurs
> + */
> + public void flushToWriter(Writer writer) throws IOException {
> + StateManager stateManager = Util.getStateManager(context);
> + Object stateToWrite = stateManager.saveView(context);
> + int totalLen = builder.length();
> + int pos = 0;
> + int tildeIdx = getNextDelimiterIndex(pos);
> + while (pos < totalLen) {
> + if (tildeIdx != -1) {
> + if (tildeIdx > pos && (tildeIdx - pos) > bufSize) {
> + // theres enough content before the first ~
> + // to fill the entire buffer
> + builder.getChars(pos, (pos + bufSize), buf, 0);
> + writer.write(buf);
> + pos += bufSize;
> } else {
> - stateManager.writeState(context, view);
> + // write all content up to the first '~'
> + builder.getChars(pos, tildeIdx, buf, 0);
> + int len = (tildeIdx - pos);
> + writer.write(buf, 0,
> len); + // now check to
> see if the state saving string is
> + // at the begining of pos, if so, write our
> + // state out.
> + if (builder.indexOf(
> + RIConstants.SAVESTATE_FIELD_MARKER,
> + pos) == tildeIdx) {
> + stateManager.writeState(context,
> stateToWrite);
> + }
> + + // push us past the
> last '~' at the end of the marker
> + pos += (len + STATE_MARKER_LEN);
> + tildeIdx = getNextDelimiterIndex(pos);
> }
> - beginIndex = markerIndex + markerLen;
> + } else {
> + // we've written all of the state field markers.
> + // finish writing content
> + if (totalLen - pos > bufSize) {
> + // there's enough content to fill the buffer
> + builder.getChars(pos, (pos + bufSize), buf, 0);
> + writer.write(buf);
> + pos += bufSize;
> + } else {
> + // we're near the end of the response
> + builder.getChars(pos, totalLen, buf, 0);
> + int len = (totalLen - pos);
> + writer.write(buf, 0, len);
> + pos += (len + 1);
> + }
> + }
> + }
> }
> - } while (-1 != markerIndex && beginIndex < contentLen);
> - } catch (Exception ex) {
> - // catch any thrown while write state.
> - throw new FacesException(ex);
> +
> + /** + * @return return the length of the
> underlying
> + * <code>StringBuilder</code>.
> + */
> + public int length() {
> + return builder.length();
> + }
> +
> + /**
> + * <p>Get the next `~' from the StringBuilder.</p>
> + * @param offset the offset from where to search from
> + * @return the index of the first '~' from the specified
> + * offset
> + */
> + private int getNextDelimiterIndex(int offset) {
> + return
> builder.indexOf(RIConstants.SAVESTATE_FIELD_DELIMITER,
> + offset); }
> + + }
> }
> Index: src/com/sun/faces/config/WebConfiguration.java
> ===================================================================
> RCS file:
> /cvs/javaserverfaces-sources/jsf-ri/src/com/sun/faces/config/WebConfiguration.java,v
>
> retrieving revision 1.4
> diff -u -r1.4 WebConfiguration.java
> --- src/com/sun/faces/config/WebConfiguration.java 22 May 2006
> 15:05:56 -0000 1.4
> +++ src/com/sun/faces/config/WebConfiguration.java 22 May 2006
> 16:23:56 -0000
> @@ -463,6 +463,10 @@
> InjectionProviderClass(
> "com.sun.faces.InjectionProvider",
> ""
> + ),
> + ResponseBufferSize(
> + "com.sun.faces.responseBufferSize",
> + "4096"
> );
>
> private String defaultValue;
> Index: src/com/sun/faces/io/FastStringWriter.java
> ===================================================================
> RCS file:
> /cvs/javaserverfaces-sources/jsf-ri/src/com/sun/faces/io/FastStringWriter.java,v
>
> retrieving revision 1.3
> diff -u -r1.3 FastStringWriter.java
> --- src/com/sun/faces/io/FastStringWriter.java 29 Mar 2006 23:03:45
> -0000 1.3
> +++ src/com/sun/faces/io/FastStringWriter.java 22 May 2006 16:23:56
> -0000
> @@ -40,7 +40,7 @@
> */
> public class FastStringWriter extends Writer {
>
> - private StringBuilder builder;
> + protected StringBuilder builder;
>
> // ------------------------------------------------------------
> Constructors
>
> Index: web/test/CorrectRenderersResponse
> ===================================================================
> RCS file:
> /cvs/javaserverfaces-sources/jsf-ri/web/test/CorrectRenderersResponse,v
> retrieving revision 1.42
> diff -u -r1.42 CorrectRenderersResponse
> --- web/test/CorrectRenderersResponse 14 Mar 2006 16:33:31 -0000
> 1.42
> +++ web/test/CorrectRenderersResponse 22 May 2006 16:23:57 -0000
> @@ -1,9 +1,9 @@
> <form id="formRenderer0" name="formRenderer0" method="post"
> action="/test/JspRedirector/root;jsessionid=52128c0171351372c7e1c01f5fd07">
>
>
> -com.sun.faces.saveStateFieldMarker<input type="hidden"
> name="formRenderer0" value="formRenderer0" /></form>
> +~com.sun.faces.saveStateFieldMarker~<input type="hidden"
> name="formRenderer0" value="formRenderer0" /></form>
> <form id="formRenderer1" name="formRenderer1" method="post"
> action="/test/JspRedirector/root;jsessionid=52128c0171351372c7e1c01f5fd07">
>
>
> -com.sun.faces.saveStateFieldMarker<input type="hidden"
> name="formRenderer1" value="formRenderer1" /></form>
> +~com.sun.faces.saveStateFieldMarker~<input type="hidden"
> name="formRenderer1" value="formRenderer1" /></form>
> <table border="0" id="radioRenderer" class="styleClass">
> <tr>
> <td>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe_at_javaserverfaces.dev.java.net
> For additional commands, e-mail: dev-help_at_javaserverfaces.dev.java.net
>