Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #1633: String.replaceAll and String.replaceFirst are known to be slow due to Pattern compiles. Prefer Pattern.matches().replaceAll() #1639

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@

package org.deegree.gml.feature;

import java.util.regex.Pattern;
import org.apache.commons.io.IOUtils;
import org.deegree.commons.tom.ResolveMode;
import org.deegree.commons.tom.ResolveParams;
Expand Down Expand Up @@ -108,6 +109,8 @@
*/
public class GMLFeatureWriterTest {

private static final Pattern NEWLINE_PATTERN = Pattern.compile("\n");

private final String SOURCE_FILE_31 = "../misc/feature/Philosopher_FeatureCollection.xml";

private final String SOURCE_FILE_32 = "../misc/feature/Philosopher_FeatureCollection_Gml32.xml";
Expand Down Expand Up @@ -596,8 +599,8 @@ private DifferenceEvaluator inoreLineBreaksInElementContent() {
Object controlValue = comparison.getControlDetails().getValue();
if (testValue instanceof String && controlValue instanceof String && testValue != null
&& testValue != null) {
String control = ((String) controlValue).replaceAll("\n", "").replace(" ", "");
String test = ((String) testValue).replaceAll("\n", "").replace(" ", "");
String control = NEWLINE_PATTERN.matcher(((String) controlValue)).replaceAll("").replace(" ", "");
String test = NEWLINE_PATTERN.matcher(((String) testValue)).replaceAll("").replace(" ", "");
return control.equals(test) ? EQUAL : DIFFERENT;
}
return comparisonResult;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,28 @@ public static String replaceFirst(String target, String from, String to) {
* @param from is the string to be replaced
* @param to is the string which will used to replace
* @return the changed target string
* @deprecated use {@link StringUtils#replaceAll(String, Pattern, String)} instead,
* with a static precompiled Pattern for better performance.
*/
@Deprecated(since = "3.6.0", forRemoval = false)
public static String replaceAll(String target, String from, String to) {
return target.replaceAll(Pattern.quote(from), Matcher.quoteReplacement(to));
}

/**
* Replaces the all substrings of this string that matches the given pattern with the
* given replacement. Works like {@link String#replaceAll(String, String)} but doesn't
* compile the pattern on every invocation.
* @param target is the original string
* @param pattern the pattern to match what should be replaced. (Static compiled for
* performance if possible)
* @param to is the string which will used to replace
* @return the changed target string
*/
public static String replaceAll(String target, Pattern pattern, String to) {
return pattern.matcher(target).replaceAll(Matcher.quoteReplacement(to));
}

/**
* Splits a string on all occurrences of delimiter and returns a list with all parts.
* Each part will be trimmed from whitespace. See
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import java.io.IOException;
import java.net.URL;

import java.util.regex.Pattern;
import org.apache.xerces.xni.XMLResourceIdentifier;
import org.apache.xerces.xni.XNIException;
import org.apache.xerces.xni.parser.XMLEntityResolver;
Expand All @@ -62,6 +63,8 @@ public class RedirectingEntityResolver implements XMLEntityResolver {

private static final URL baseURL;

private static final Pattern HTTP_PATTERN = Pattern.compile("http://");

static {
baseURL = RedirectingEntityResolver.class.getResource(ROOT);
if (baseURL == null) {
Expand All @@ -87,7 +90,7 @@ public String redirect(String systemId) {
}
}
else if (systemId.startsWith(INSPIRE_SCHEMAS_URL)) {
return systemId.replaceFirst("http://", "https://");
return HTTP_PATTERN.matcher(systemId).replaceFirst("https://");
}
else if (systemId.equals("http://www.w3.org/2001/xml.xsd")) {
// workaround for schemas that include the xml base schema...
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@

import java.util.List;

import java.util.regex.Pattern;
import org.junit.Test;

/**
Expand All @@ -49,25 +50,27 @@
*/
public class StringToolsTest {

private static final Pattern QUOTE_PATTERN = Pattern.compile(Pattern.quote("*"));

/**
* Test method for
* {@link org.deegree.commons.utils.StringUtils#replaceAll(java.lang.String, java.lang.String, java.lang.String)}.
* {@link org.deegree.commons.utils.StringUtils#replaceAll(java.lang.String, java.util.regex.Pattern, java.lang.String)}.
*/
@Test
public void testReplaceAll() {
assertEquals("foo|bar||baz", StringUtils.replaceAll("foo*bar**baz", "*", "|"));
assertEquals("foo$1bar$1baz", StringUtils.replaceAll("foo*bar*baz", "*", "$1"));
assertEquals("", StringUtils.replaceAll("", "*", "$1"));
assertEquals("foo|bar||baz", StringUtils.replaceAll("foo*bar**baz", QUOTE_PATTERN, "|"));
assertEquals("foo$1bar$1baz", StringUtils.replaceAll("foo*bar*baz", QUOTE_PATTERN, "$1"));
assertEquals("", StringUtils.replaceAll("", QUOTE_PATTERN, "$1"));
}

/**
* Test method for
* {@link org.deegree.commons.utils.StringUtils#replaceAll(java.lang.String, java.lang.String, java.lang.String)}.
* {@link org.deegree.commons.utils.StringUtils#replaceAll(java.lang.String, java.util.regex.Pattern, java.lang.String)}.
*/
@Test(expected = NullPointerException.class)
public void testReplaceAllNull() {
// should that be the correct behaviour? or should it return an empty string?
StringUtils.replaceAll(null, "*", "$1");
StringUtils.replaceAll(null, QUOTE_PATTERN, "$1");
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@

package org.deegree.coverage.rangeset;

import java.util.regex.Pattern;

/**
* The <code>Interval</code> an intervall.
*
Expand Down Expand Up @@ -86,6 +88,9 @@ public enum Closure {
/** simple boundary representation of the end of an interval */
public final String end;

/** precompiled pattern for matching the character - */
private static final Pattern DASH_PATTERN = Pattern.compile("-");

private Closure(String begin, String end) {
this.begin = begin;
this.end = end;
Expand All @@ -98,7 +103,7 @@ private Closure(String begin, String end) {
public static Closure fromString(String closureValue) {
Closure result = Closure.closed;
if (closureValue != null && !"".equals(closureValue)) {
String mapped = closureValue.replaceAll("-", "_");
String mapped = DASH_PATTERN.matcher(closureValue).replaceAll("_");
try {
result = Closure.valueOf(mapped.toLowerCase());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;

import java.util.regex.Pattern;
import org.deegree.commons.utils.FileUtils;
import org.deegree.coverage.raster.AbstractRaster;
import org.deegree.coverage.raster.data.container.BufferResult;
Expand Down Expand Up @@ -78,6 +79,14 @@ public class CacheRasterReader extends GridFileReader {

private static final int TILE_SIZE = 500;

private static final Pattern CURLY_BRACKET_OPEN_PATTERN = Pattern.compile("\\{");

private static final Pattern CURLY_BRACKET_CLOSE_PATTERN = Pattern.compile("\\}");

private static final Pattern SEMICOLON_PATTERN = Pattern.compile("\\:");

private static final Pattern WHITESPACE_PATTERN = Pattern.compile("\\s");

private final Object LOCK = new Object();

private final Map<Integer, TileEntry> tiles;
Expand Down Expand Up @@ -498,10 +507,10 @@ private String createId(int width, int height, RasterDataInfo rdi, RasterGeoRefe
sb.append("_w_").append(width);
sb.append("_h_").append(height);
String result = sb.toString();
result = result.replaceAll("\\{", "_");
result = result.replaceAll("\\}", "_");
result = result.replaceAll("\\:", "_");
result = result.replaceAll("\\s", "_");
result = CURLY_BRACKET_OPEN_PATTERN.matcher(result).replaceAll("_");
result = CURLY_BRACKET_CLOSE_PATTERN.matcher(result).replaceAll("_");
result = SEMICOLON_PATTERN.matcher(result).replaceAll("_");
result = WHITESPACE_PATTERN.matcher(result).replaceAll("_");
return result;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import java.awt.image.PixelInterleavedSampleModel;
import java.awt.image.SampleModel;

import java.util.regex.Pattern;
import org.deegree.coverage.raster.data.RasterData;

/**
Expand Down Expand Up @@ -121,6 +122,10 @@ public enum BandType {
*/
public final static BandType[] RGBA = new BandType[] { RED, GREEN, BLUE, ALPHA };

private static final Pattern DASH_PATTERN = Pattern.compile("-");

private static final Pattern WHITESPACE_PATTERN = Pattern.compile("\\s");

private final String info;

BandType(String info) {
Expand Down Expand Up @@ -195,8 +200,8 @@ public static BandType[] fromBufferedImageType(int type, int expectedSize, Sampl
*/
public static BandType fromString(String band) {
String key = band.toUpperCase();
key = key.replaceAll("-", "_");
key = key.replaceAll("\\s", "_");
key = DASH_PATTERN.matcher(key).replaceAll("_");
key = WHITESPACE_PATTERN.matcher(key).replaceAll("_");
BandType bt = null;
try {
bt = BandType.valueOf(key);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
----------------------------------------------------------------------------*/
package org.deegree.coverage.raster.interpolation;

import java.util.regex.Pattern;

/**
* Enum for all implemented interpolation types.
*
Expand All @@ -48,6 +50,10 @@ public enum InterpolationType {
/** No interpolation */
NONE;

private static final Pattern DASH_PATTERN = Pattern.compile("-");

private static final Pattern WHITESPACE_PATTERN = Pattern.compile("\\s");

/**
* Get interpolation for the given string. This method is case insensitive, words can
* be separated with a whitespace, minus or underscore.
Expand All @@ -56,8 +62,8 @@ public enum InterpolationType {
*/
public static InterpolationType fromString(String interpolation) {
String key = interpolation.toUpperCase();
key = key.replaceAll("-", "_");
key = key.replaceAll("\\s", "_");
key = DASH_PATTERN.matcher(key).replaceAll("_");
key = WHITESPACE_PATTERN.matcher(key).replaceAll("_");
return InterpolationType.valueOf(key);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
import java.util.List;
import java.util.Map;

import java.util.regex.Pattern;
import javax.vecmath.Point2d;

import org.deegree.cs.CRSCodeType;
Expand Down Expand Up @@ -102,6 +103,8 @@ public class WKTParser {

private static final Logger LOG = getLogger(WKTParser.class);

private static final Pattern UNDERSCORE_PATTERN = Pattern.compile("_");

private StreamTokenizer tokenizer;

private Reader buff;
Expand All @@ -114,15 +117,15 @@ public class WKTParser {
*/
protected boolean equalsParameterVariants(String candidate, String paramName) {
String candidateVariant = candidate.replace("_", "");
String paramVariant = paramName.replaceAll("_", "");
String paramVariant = UNDERSCORE_PATTERN.matcher(paramName).replaceAll("");
if (candidateVariant.equalsIgnoreCase(paramVariant)) {
return true;
}
return false;
}

protected String makeInvariantKey(String candidate) {
return candidate.replaceAll("_", "").toLowerCase();
return UNDERSCORE_PATTERN.matcher(candidate).replaceAll("").toLowerCase();
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import java.util.ArrayList;
import java.util.List;

import java.util.regex.Pattern;
import org.deegree.commons.tom.TypedObjectNode;
import org.deegree.commons.tom.primitive.PrimitiveType;
import org.deegree.commons.tom.primitive.PrimitiveValue;
Expand Down Expand Up @@ -117,6 +118,18 @@ public abstract class AbstractWhereBuilder {

private static final Logger LOG = LoggerFactory.getLogger(AbstractWhereBuilder.class);

private static final String WILDCARD = "*";

private static final String SINGLECHAR = "?";

private static final String ESCAPECHAR = "\\";

private static final Pattern ESCAPE_CHAR_PATTERN = Pattern.compile(Pattern.quote(ESCAPECHAR));

private static final Pattern SINGLE_CHAR_PATTERN = Pattern.compile(Pattern.quote(SINGLECHAR));

private static final Pattern WILDCARD_PATTERN = Pattern.compile(Pattern.quote(WILDCARD));

protected final SQLDialect dialect;

protected final PropertyNameMapper mapper;
Expand Down Expand Up @@ -540,15 +553,12 @@ private PropertyIsLike buildIsLike(Expression propName, Expression literal, bool
throw new UnmappableException(msg);
}

String wildCard = "*";
String singleChar = "?";
String escapeChar = "\\";
String s = ((Literal<?>) literal).getValue().toString();
s = StringUtils.replaceAll(s, escapeChar, escapeChar + escapeChar);
s = StringUtils.replaceAll(s, singleChar, escapeChar + singleChar);
s = StringUtils.replaceAll(s, wildCard, escapeChar + wildCard);
s = StringUtils.replaceAll(s, ESCAPE_CHAR_PATTERN, ESCAPECHAR + ESCAPECHAR);
s = StringUtils.replaceAll(s, SINGLE_CHAR_PATTERN, ESCAPECHAR + SINGLECHAR);
s = StringUtils.replaceAll(s, WILDCARD_PATTERN, ESCAPECHAR + WILDCARD);
Literal<PrimitiveValue> escapedLiteral = new Literal<PrimitiveValue>(new PrimitiveValue(s), null);
return new PropertyIsLike((ValueReference) propName, escapedLiteral, wildCard, singleChar, escapeChar,
return new PropertyIsLike((ValueReference) propName, escapedLiteral, WILDCARD, SINGLECHAR, ESCAPECHAR,
matchCase, null);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@

public class TrueTypeFontLoader implements WellKnownNameLoader {

private static final Pattern FONT_PREFIX_PATTERN = Pattern.compile("[tT][tT][fF]://(.*)#(.*)");

private static final Logger LOG = LoggerFactory.getLogger(TrueTypeFontLoader.class);

private static final String PREFIX = "ttf://";
Expand All @@ -52,7 +54,7 @@ public Shape parse(String wellKnownName, Function<String, URL> resolver) {
if (wellKnownName == null || !wellKnownName.startsWith(PREFIX))
return null;

Matcher m = Pattern.compile("[tT][tT][fF]://(.*)#(.*)").matcher(wellKnownName);
Matcher m = FONT_PREFIX_PATTERN.matcher(wellKnownName);
if (!m.matches()) {
throw new IllegalArgumentException("Invalid WellKnownName, use syntax ttf://<fontName>#<charCode>");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ class IdUtils {

private static final Logger LOG = getLogger(IdUtils.class);

private static final Pattern NUMBER_PATTERN = Pattern.compile("[0-9]");

private final Connection conn;

private String mainTable;
Expand Down Expand Up @@ -96,8 +98,7 @@ String generateUUID() throws MetadataInspectorException {

uuid = UUID.randomUUID().toString();
char firstChar = uuid.charAt(0);
Pattern p = Pattern.compile("[0-9]");
Matcher m = p.matcher("" + firstChar);
Matcher m = NUMBER_PATTERN.matcher("" + firstChar);
if (m.matches()) {
int i;
double ma = Math.random();
Expand All @@ -110,7 +111,7 @@ String generateUUID() throws MetadataInspectorException {
}

firstChar = (char) ((int) (i + ma * 26));
uuid = uuid.replaceFirst("[0-9]", String.valueOf(firstChar));
uuid = NUMBER_PATTERN.matcher(uuid).replaceFirst(String.valueOf(firstChar));
}
boolean uuidIsEqual = false;

Expand Down Expand Up @@ -146,12 +147,8 @@ String generateUUID() throws MetadataInspectorException {
boolean checkUUIDCompliance(String uuid) {

char firstChar = uuid.charAt(0);
Pattern p = Pattern.compile("[0-9]");
Matcher m = p.matcher("" + firstChar);
if (m.matches()) {
return false;
}
return true;
Matcher m = NUMBER_PATTERN.matcher("" + firstChar);
return !m.matches();
}

boolean checkUUIDCompliance(List<String> list) {
Expand Down
Loading