Skip to content

Commit

Permalink
Optimize allocation in StringUtils#cleanPath
Browse files Browse the repository at this point in the history
This commit applies several optimizations to StringUtils#cleanPath and
related methods:

* pre-size pathElements deque in StringUtils#cleanPath with
  pathElements.length elements, since this this is the maximum size and
  the most likely case.
* optimize StringUtils#collectionToDelimitedString to calculate the size
  of the resulting String and avoid array auto-resizing in the
  StringBuilder.
* If the path did not contain any components that required cleaning,
  return the (normalized) path as-is. No need to concatenate the prefix
  and the trailing path.

See spring-projectsgh-26316
  • Loading branch information
knittl authored and lxbzmy committed Mar 26, 2022
1 parent 4c502e0 commit 49530dd
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 4 deletions.
Expand Up @@ -667,7 +667,9 @@ public static String cleanPath(String path) {
if (!hasLength(path)) {
return path;
}
String pathToUse = replace(path, WINDOWS_FOLDER_SEPARATOR, FOLDER_SEPARATOR);

String normalizedPath = replace(path, WINDOWS_FOLDER_SEPARATOR, FOLDER_SEPARATOR);
String pathToUse = normalizedPath;

// Shortcut if there is no work to do
if (pathToUse.indexOf('.') == -1) {
Expand Down Expand Up @@ -695,7 +697,8 @@ public static String cleanPath(String path) {
}

String[] pathArray = delimitedListToStringArray(pathToUse, FOLDER_SEPARATOR);
Deque<String> pathElements = new ArrayDeque<>();
// we never require more elements than pathArray and in the common case the same number
Deque<String> pathElements = new ArrayDeque<>(pathArray.length);
int tops = 0;

for (int i = pathArray.length - 1; i >= 0; i--) {
Expand All @@ -721,7 +724,7 @@ else if (TOP_PATH.equals(element)) {

// All path elements stayed the same - shortcut
if (pathArray.length == pathElements.size()) {
return prefix + pathToUse;
return normalizedPath;
}
// Remaining top paths need to be retained.
for (int i = 0; i < tops; i++) {
Expand All @@ -732,7 +735,40 @@ else if (TOP_PATH.equals(element)) {
pathElements.addFirst(CURRENT_PATH);
}

return prefix + collectionToDelimitedString(pathElements, FOLDER_SEPARATOR);
final String joined = joinStrings(pathElements, FOLDER_SEPARATOR);
// avoid string concatenation with empty prefix
return prefix.isEmpty() ? joined : prefix + joined;
}

/**
* Convert a {@link Collection Collection&lt;String&gt;} to a delimited {@code String} (e.g. CSV).
* <p>This is an optimized variant of {@link #collectionToDelimitedString(Collection, String)}, which does not
* require dynamic resizing of the StringBuilder's backing array.
* @param coll the {@code Collection Collection&lt;String&gt;} to convert (potentially {@code null} or empty)
* @param delim the delimiter to use (typically a ",")
* @return the delimited {@code String}
*/
private static String joinStrings(@Nullable Collection<String> coll, String delim) {

if (CollectionUtils.isEmpty(coll)) {
return "";
}

// precompute total length of resulting string
int totalLength = (coll.size() - 1) * delim.length();
for (String str : coll) {
totalLength += str.length();
}

StringBuilder sb = new StringBuilder(totalLength);
Iterator<?> it = coll.iterator();
while (it.hasNext()) {
sb.append(it.next());
if (it.hasNext()) {
sb.append(delim);
}
}
return sb.toString();
}

/**
Expand Down
Expand Up @@ -402,6 +402,8 @@ void cleanPath() {
assertThat(StringUtils.cleanPath("file:.././")).isEqualTo("file:../");
assertThat(StringUtils.cleanPath("file:/mypath/spring.factories")).isEqualTo("file:/mypath/spring.factories");
assertThat(StringUtils.cleanPath("file:///c:/some/../path/the%20file.txt")).isEqualTo("file:///c:/path/the%20file.txt");
assertThat(StringUtils.cleanPath("jar:file:///c:\\some\\..\\path\\.\\the%20file.txt")).isEqualTo("jar:file:///c:/path/the%20file.txt");
assertThat(StringUtils.cleanPath("jar:file:///c:/some/../path/./the%20file.txt")).isEqualTo("jar:file:///c:/path/the%20file.txt");
}

@Test
Expand Down

0 comments on commit 49530dd

Please sign in to comment.