This is an automated email from the ASF dual-hosted git repository.
adelbene pushed a commit to branch wicket-9.x
in repository https://gitbox.apache.org/repos/asf/wicket.git
The following commit(s) were added to refs/heads/wicket-9.x by this push:
new f14aa98da2 WICKET-6992 Reduce object creation and wasted memory in Url
toString() methods
f14aa98da2 is described below
commit f14aa98da2839e027a31e7cc36459438016afe4a
Author: Alan Stange <[email protected]>
AuthorDate: Wed May 4 22:07:24 2022 -0400
WICKET-6992 Reduce object creation and wasted memory in Url toString()
methods
The Url class generates a lot of StringBuilders and temporary buffers.
It's a bit frustrating form a performance
standpoint as the builders often need to resize, further compounding the
performance impact.
- QueryParameter.toString(...) can be updated to not generate any
extraneous garbage objects using JEP 280 String
concatenation. This is faster than the original code and avoids extra
garbage object creation.
- enhance Url.getPath() by using getPathInternal() which allows the code to
return an optimal result. "" for no segments;
the String for 1 segment, and the resulting StringBuilder for more than 2
segments. This avoids copying the internal byte[]/char[]
array yet again and wasting more memory. The callers of this method can
then use the returned object in the way that best
suits them. NB the case for two segments should probably also be handled
by explicit invocation of
seg1 + / + seg2 as is would be faster and save memory.
Note that we now compute the size of the StringBuilder in advance, avoiding
the repeated resizing of the builder that had been
happening in the original code.
- enhance getQueryString() in a similar manner. Handle the n=0 and n=1
common cases without generating any extra garbage.
Size the StringBuilder a little better in this case as well. Again, the
n=2 case should probably with String concatenation.
The Url.toString(mode, charset) method is a bit more complicated. It is
rarely called (if ever?) with StringMode=FULL and it
appears as if fragment is commonly unset as well, so handle that 99% code
path at the top efficiently without any
wasted memory. If not, then fall through into the original code. The
call to getPathInternal here avoids making a copy of
that array.
Note that it might be much more efficient to allow this code to pass a
StringBuilder instance around. This would avoid some
repeated copying of byte[]/char[] arrays. These changes here greatly
reduce the memory waste, but more can still be done.
I'm a bit surprised about the missing ). Must have fat fingered it at the
last second.
I'm a bit surprised about the missing ). Must have fat fingered it at the
last second.
I'm not a fan of the brace style used in this project... ;-)
Two changes in response to the feedback.
- cache the result of a function call
- use charAt() and avoid a toString() call.
Fix code formatting.
---
.../main/java/org/apache/wicket/request/Url.java | 96 ++++++++++++++++------
.../apache/wicket/util/encoding/UrlEncoder.java | 2 +-
2 files changed, 70 insertions(+), 28 deletions(-)
diff --git a/wicket-request/src/main/java/org/apache/wicket/request/Url.java
b/wicket-request/src/main/java/org/apache/wicket/request/Url.java
index 8ccc8e8d99..ab505f5497 100755
--- a/wicket-request/src/main/java/org/apache/wicket/request/Url.java
+++ b/wicket-request/src/main/java/org/apache/wicket/request/Url.java
@@ -747,8 +747,28 @@ public class Url implements Serializable
*/
public String toString(StringMode mode, Charset charset)
{
- StringBuilder result = new StringBuilder();
- final String path = getPath(charset);
+ // this method is rarely called with StringMode == FULL.
+
+ final CharSequence path = getPathInternal(charset);
+ final String queryString = getQueryString(charset);
+ String _fragment = getFragment();
+
+ // short circuit all the processing in the most common cases
+ if (StringMode.FULL != mode && Strings.isEmpty(_fragment))
+ {
+ if (queryString == null)
+ {
+ return path.toString();
+ }
+ else
+ {
+ return path + "?" + queryString;
+ }
+ }
+
+ // fall through into the traditional code path
+
+ StringBuilder result = new StringBuilder(64);
if (StringMode.FULL == mode)
{
@@ -781,22 +801,19 @@ public class Url implements Serializable
StringMode.FULL.name() + " mode because
it has a `..` segment: " + toString());
}
- if (!path.startsWith("/"))
+ if (!path.isEmpty() && !(path.charAt(0) == '/'))
{
result.append('/');
}
-
}
result.append(path);
- final String queryString = getQueryString(charset);
if (queryString != null)
{
result.append('?').append(queryString);
}
- String _fragment = getFragment();
if (Strings.isEmpty(_fragment) == false)
{
result.append('#').append(_fragment);
@@ -1021,14 +1038,15 @@ public class Url implements Serializable
*/
public String toString(final Charset charset)
{
- StringBuilder result = new StringBuilder();
- result.append(encodeParameter(getName(), charset));
- if (!Strings.isEmpty(getValue()))
+ String value = getValue();
+ if (Strings.isEmpty(value))
{
- result.append('=');
- result.append(encodeParameter(getValue(),
charset));
+ return encodeParameter(getName(), charset);
+ }
+ else
+ {
+ return encodeParameter(getName(), charset) +
"=" + encodeParameter(value, charset);
}
- return result.toString();
}
}
@@ -1202,10 +1220,35 @@ public class Url implements Serializable
* @return path string
*/
public String getPath(Charset charset)
+ {
+ return getPathInternal(charset).toString();
+ }
+
+ /**
+ * return path for current url in given encoding, with optimizations
for common use to avoid excessive object creation
+ * and resizing of StringBuilders. Used internally by Url
+ *
+ * @param charset
+ * character set for encoding
+ *
+ * @return path string
+ */
+ private CharSequence getPathInternal(Charset charset)
{
Args.notNull(charset, "charset");
- StringBuilder path = new StringBuilder();
+ List<String> segments = getSegments();
+ // these two common cases can be handled with no additional
overhead, so do that.
+ if (segments.isEmpty())
+ return "";
+ if (segments.size() == 1)
+ return encodeSegment(segments.get(0), charset);
+
+ int length = 0;
+ for (String segment : getSegments())
+ length += segment.length() + 4;
+
+ StringBuilder path = new StringBuilder(length);
boolean slash = false;
for (String segment : getSegments())
@@ -1217,7 +1260,7 @@ public class Url implements Serializable
path.append(encodeSegment(segment, charset));
slash = true;
}
- return path.toString();
+ return path;
}
/**
@@ -1243,24 +1286,23 @@ public class Url implements Serializable
{
Args.notNull(charset, "charset");
- String queryString = null;
List<QueryParameter> queryParameters = getQueryParameters();
+ if (queryParameters.isEmpty())
+ return null;
+ if (queryParameters.size() == 1)
+ return queryParameters.get(0).toString(charset);
- if (queryParameters.size() != 0)
+ // make a reasonable guess at a size for this builder
+ StringBuilder query = new StringBuilder(16 * parameters.size());
+ for (QueryParameter parameter : queryParameters)
{
- StringBuilder query = new StringBuilder();
-
- for (QueryParameter parameter : queryParameters)
- {
- if (query.length() != 0)
- {
- query.append('&');
- }
- query.append(parameter.toString(charset));
+ if (query.length() != 0) {
+ query.append('&');
}
- queryString = query.toString();
+ query.append(parameter.toString(charset));
}
- return queryString;
+
+ return query.toString();
}
/**
diff --git
a/wicket-util/src/main/java/org/apache/wicket/util/encoding/UrlEncoder.java
b/wicket-util/src/main/java/org/apache/wicket/util/encoding/UrlEncoder.java
index 7fa4b6709f..299fb8b693 100644
--- a/wicket-util/src/main/java/org/apache/wicket/util/encoding/UrlEncoder.java
+++ b/wicket-util/src/main/java/org/apache/wicket/util/encoding/UrlEncoder.java
@@ -249,7 +249,7 @@ public class UrlEncoder
}
}
}
- return new String(bos.toByteArray(), charset);
+ return bos.toString(charset);
}
}
