You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@myfaces.apache.org by ma...@apache.org on 2008/07/04 10:54:27 UTC
svn commit: r673964 - in /myfaces/trinidad/branches/1.2.8.1-branch:
trinidad-api/src/main/java/org/apache/myfaces/trinidad/util/
trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/renderkit/core/xhtml/
trinidad-impl/src/main/java/org/apach...
Author: matzew
Date: Fri Jul 4 01:54:26 2008
New Revision: 673964
URL: http://svn.apache.org/viewvc?rev=673964&view=rev
Log:
TRINIDAD-1126 - Contention at java.util.regex.Pattern.matcher
Problem is, String.replace(CharSeq, CharSeq) uses internally Pattern.matcher, which has a (serious) bug, b/c it does synchronized(this), which causes PERF issues.
JDK bug is known, already:
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6436458
Not sure if it is fixed in an update of Java5 or 6, but it is fixed in Java7.
For what it's worth, Apache Harmony, doesn't have that problem.
Fix:
-Introduced a StringUtils.replace(String, String, String), taken from Commons LANG.
-used this static util, instead of "nativ" java language feature(the String.replace(CharSq, CharSq)).
-In StyleUtils we got rid of Pattern.matcher().replaceAll. This is exactly, what the String.replace() in question does. Using the "new", borrowed, utility here as well
ported the fix to the 1.2.8.1 branch as well
Added:
myfaces/trinidad/branches/1.2.8.1-branch/trinidad-api/src/main/java/org/apache/myfaces/trinidad/util/StringUtils.java
Modified:
myfaces/trinidad/branches/1.2.8.1-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/renderkit/core/xhtml/GoButtonRenderer.java
myfaces/trinidad/branches/1.2.8.1-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/util/StyleUtils.java
myfaces/trinidad/branches/1.2.8.1-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/util/MimeUtility.java
Added: myfaces/trinidad/branches/1.2.8.1-branch/trinidad-api/src/main/java/org/apache/myfaces/trinidad/util/StringUtils.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/branches/1.2.8.1-branch/trinidad-api/src/main/java/org/apache/myfaces/trinidad/util/StringUtils.java?rev=673964&view=auto
==============================================================================
--- myfaces/trinidad/branches/1.2.8.1-branch/trinidad-api/src/main/java/org/apache/myfaces/trinidad/util/StringUtils.java (added)
+++ myfaces/trinidad/branches/1.2.8.1-branch/trinidad-api/src/main/java/org/apache/myfaces/trinidad/util/StringUtils.java Fri Jul 4 01:54:26 2008
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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.apache.myfaces.trinidad.util;
+
+/**
+ * This class contains String utilities. The code was taken from
+ * Apache Commons, LANG project.
+ *
+ * Reason is to provide an alternate method to java.lang.String.replace().
+ * That method uses java.util.regex.Pattern.matcher(), which has a PERF
+ * issue.
+ *
+ */
+public final class StringUtils
+{
+ private StringUtils()
+ {
+ // no-op
+ }
+
+ public static String replace(String text, String searchString, String replacement)
+ {
+ return replace(text, searchString, replacement, -1);
+ }
+
+ // making this public, doesn't hurt... but we don't need it, now
+ private static boolean isEmpty(String str)
+ {
+ return str == null || str.length() == 0;
+ }
+
+ // making this public, doesn't hurt... but we don't need it, now
+ private static String replace(String text, String searchString, String replacement, int max)
+ {
+ if (isEmpty(text) || isEmpty(searchString) || replacement == null || max == 0)
+ {
+ return text;
+ }
+ int start = 0;
+ int end = text.indexOf(searchString, start);
+
+ if (end == -1)
+ {
+ return text;
+ }
+ int replLength = searchString.length();
+ int increase = replacement.length() - replLength;
+ increase = (increase < 0 ? 0 : increase);
+ increase *= (max < 0 ? 16 : (max > 64 ? 64 : max));
+ StringBuffer buf = new StringBuffer(text.length() + increase);
+ while (end != -1)
+ {
+ buf.append(text.substring(start, end)).append(replacement);
+ start = end + replLength;
+
+ if (--max == 0)
+ {
+ break;
+ }
+ end = text.indexOf(searchString, start);
+ }
+ buf.append(text.substring(start));
+ return buf.toString();
+ }
+}
Modified: myfaces/trinidad/branches/1.2.8.1-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/renderkit/core/xhtml/GoButtonRenderer.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/branches/1.2.8.1-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/renderkit/core/xhtml/GoButtonRenderer.java?rev=673964&r1=673963&r2=673964&view=diff
==============================================================================
--- myfaces/trinidad/branches/1.2.8.1-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/renderkit/core/xhtml/GoButtonRenderer.java (original)
+++ myfaces/trinidad/branches/1.2.8.1-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/renderkit/core/xhtml/GoButtonRenderer.java Fri Jul 4 01:54:26 2008
@@ -27,6 +27,7 @@
import javax.faces.context.FacesContext;
import javax.faces.context.ResponseWriter;
+import org.apache.myfaces.trinidad.util.StringUtils;
import org.apache.myfaces.trinidad.bean.FacesBean;
import org.apache.myfaces.trinidad.bean.PropertyKey;
import org.apache.myfaces.trinidad.component.core.nav.CoreGoButton;
@@ -280,7 +281,7 @@
else
{
// Escape the destination in case there's any quotes
- destination = destination.replace("'", "\\'");
+ destination = StringUtils.replace(destination, "'", "\\'");
String targetFrame = getTargetFrame(bean);
// Look for target frames with well-known names, like
Modified: myfaces/trinidad/branches/1.2.8.1-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/util/StyleUtils.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/branches/1.2.8.1-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/util/StyleUtils.java?rev=673964&r1=673963&r2=673964&view=diff
==============================================================================
--- myfaces/trinidad/branches/1.2.8.1-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/util/StyleUtils.java (original)
+++ myfaces/trinidad/branches/1.2.8.1-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/style/util/StyleUtils.java Fri Jul 4 01:54:26 2008
@@ -18,7 +18,7 @@
*/
package org.apache.myfaces.trinidadinternal.style.util;
-import java.util.regex.Pattern;
+import org.apache.myfaces.trinidad.util.StringUtils;
/**
* Generic style utilities.
@@ -44,13 +44,12 @@
if (selector == null) return null;
selector = selector.replace('|', '_');
- // This is the code that's actually getting called by String.replaceAll();
- // this code is way too heavy.
- if (selector.indexOf("::") > 0)
- selector = _DOUBLE_COLON_PATTERN.matcher(selector).replaceAll("_");
+
+ if (selector.indexOf(_DOUBLE_COLON) > 0)
+ selector = StringUtils.replace(selector, _DOUBLE_COLON, "_");
return selector;
}
- static private final Pattern _DOUBLE_COLON_PATTERN = Pattern.compile("::");
+ static private final String _DOUBLE_COLON = "::";
}
Modified: myfaces/trinidad/branches/1.2.8.1-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/util/MimeUtility.java
URL: http://svn.apache.org/viewvc/myfaces/trinidad/branches/1.2.8.1-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/util/MimeUtility.java?rev=673964&r1=673963&r2=673964&view=diff
==============================================================================
--- myfaces/trinidad/branches/1.2.8.1-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/util/MimeUtility.java (original)
+++ myfaces/trinidad/branches/1.2.8.1-branch/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/util/MimeUtility.java Fri Jul 4 01:54:26 2008
@@ -22,6 +22,8 @@
import java.net.URLEncoder;
+import org.apache.myfaces.trinidad.util.StringUtils;
+
final public class MimeUtility
{
@@ -40,7 +42,7 @@
try
{
// IE does not understand "+ = space", workaround here
- return URLEncoder.encode(word, "UTF-8").replace("+", "%20");
+ return StringUtils.replace(URLEncoder.encode(word, "UTF-8"), "+", "%20");
}
catch (UnsupportedEncodingException e)
{