Skip to content

Commit cc026fc

Browse files
committed
Polish "Optimize allocation in StringUtils#cleanPath"
This commit also introduces JMH benchmarks related to the code optimizations. Closes gh-2631
1 parent 8d3e8ca commit cc026fc

File tree

2 files changed

+123
-33
lines changed

2 files changed

+123
-33
lines changed
Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
/*
2+
* Copyright 2002-2021 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.util;
18+
19+
import java.util.ArrayList;
20+
import java.util.Arrays;
21+
import java.util.Collection;
22+
import java.util.List;
23+
import java.util.Random;
24+
25+
import org.openjdk.jmh.annotations.Benchmark;
26+
import org.openjdk.jmh.annotations.BenchmarkMode;
27+
import org.openjdk.jmh.annotations.Level;
28+
import org.openjdk.jmh.annotations.Mode;
29+
import org.openjdk.jmh.annotations.Param;
30+
import org.openjdk.jmh.annotations.Scope;
31+
import org.openjdk.jmh.annotations.Setup;
32+
import org.openjdk.jmh.annotations.State;
33+
import org.openjdk.jmh.infra.Blackhole;
34+
35+
/**
36+
* Benchmarks for {@link StringUtils}.
37+
*
38+
* @author Brian Clozel
39+
*/
40+
@BenchmarkMode(Mode.Throughput)
41+
public class StringUtilsBenchmark {
42+
43+
@Benchmark
44+
public void collectionToDelimitedString(DelimitedStringState state, Blackhole bh) {
45+
bh.consume(StringUtils.collectionToCommaDelimitedString(state.elements));
46+
}
47+
48+
@State(Scope.Benchmark)
49+
public static class DelimitedStringState {
50+
51+
@Param("10")
52+
int elementMinSize;
53+
54+
@Param("20")
55+
int elementMaxSize;
56+
57+
@Param("10")
58+
int elementCount;
59+
60+
Collection<String> elements;
61+
62+
@Setup(Level.Iteration)
63+
public void setup() {
64+
Random random = new Random();
65+
this.elements = new ArrayList<>(this.elementCount);
66+
int bound = this.elementMaxSize - this.elementMinSize;
67+
for (int i = 0; i < this.elementCount; i++) {
68+
this.elements.add(String.format("%0" + (random.nextInt(bound) + this.elementMinSize) + "d", 1));
69+
}
70+
}
71+
}
72+
73+
@Benchmark
74+
public void cleanPath(CleanPathState state, Blackhole bh) {
75+
for (String path : state.paths) {
76+
bh.consume(StringUtils.cleanPath(path));
77+
}
78+
}
79+
80+
@State(Scope.Benchmark)
81+
public static class CleanPathState {
82+
83+
private static final List<String> SEGMENTS = Arrays.asList("some", "path", ".", "..", "springspring");
84+
85+
@Param("10")
86+
int segmentCount;
87+
88+
@Param("20")
89+
int pathsCount;
90+
91+
Collection<String> paths;
92+
93+
@Setup(Level.Iteration)
94+
public void setup() {
95+
this.paths = new ArrayList<>(this.pathsCount);
96+
Random random = new Random();
97+
for (int i = 0; i < this.pathsCount; i++) {
98+
this.paths.add(createSamplePath(random));
99+
}
100+
}
101+
102+
private String createSamplePath(Random random) {
103+
String separator = random.nextBoolean() ? "/" : "\\";
104+
StringBuilder sb = new StringBuilder();
105+
sb.append("jar:file:///c:");
106+
for (int i = 0; i < this.segmentCount; i++) {
107+
sb.append(separator);
108+
sb.append(SEGMENTS.get(random.nextInt(SEGMENTS.size())));
109+
}
110+
sb.append(separator);
111+
sb.append("the%20file.txt");
112+
return sb.toString();
113+
}
114+
115+
}
116+
}

spring-core/src/main/java/org/springframework/util/StringUtils.java

Lines changed: 7 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -735,42 +735,11 @@ else if (TOP_PATH.equals(element)) {
735735
pathElements.addFirst(CURRENT_PATH);
736736
}
737737

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

743-
/**
744-
* Convert a {@link Collection Collection&lt;String&gt;} to a delimited {@code String} (e.g. CSV).
745-
* <p>This is an optimized variant of {@link #collectionToDelimitedString(Collection, String)}, which does not
746-
* require dynamic resizing of the StringBuilder's backing array.
747-
* @param coll the {@code Collection Collection&lt;String&gt;} to convert (potentially {@code null} or empty)
748-
* @param delim the delimiter to use (typically a ",")
749-
* @return the delimited {@code String}
750-
*/
751-
private static String joinStrings(@Nullable Collection<String> coll, String delim) {
752-
753-
if (CollectionUtils.isEmpty(coll)) {
754-
return "";
755-
}
756-
757-
// precompute total length of resulting string
758-
int totalLength = (coll.size() - 1) * delim.length();
759-
for (String str : coll) {
760-
totalLength += str.length();
761-
}
762-
763-
StringBuilder sb = new StringBuilder(totalLength);
764-
Iterator<?> it = coll.iterator();
765-
while (it.hasNext()) {
766-
sb.append(it.next());
767-
if (it.hasNext()) {
768-
sb.append(delim);
769-
}
770-
}
771-
return sb.toString();
772-
}
773-
774743
/**
775744
* Compare two paths after normalization of them.
776745
* @param path1 first path for comparison
@@ -1330,7 +1299,12 @@ public static String collectionToDelimitedString(
13301299
return "";
13311300
}
13321301

1333-
StringBuilder sb = new StringBuilder();
1302+
int totalLength = coll.size() * (prefix.length() + suffix.length()) + (coll.size() - 1) * delim.length();
1303+
for (Object element : coll) {
1304+
totalLength += element.toString().length();
1305+
}
1306+
1307+
StringBuilder sb = new StringBuilder(totalLength);
13341308
Iterator<?> it = coll.iterator();
13351309
while (it.hasNext()) {
13361310
sb.append(prefix).append(it.next()).append(suffix);

0 commit comments

Comments
 (0)