related issue: #29611
RequestMappingInfoHandlerMapping handleNoMatch() method has the following format.
https://github.com/spring-projects/spring-framework/blob/main/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMapping.java#L246
https://github.com/spring-projects/spring-framework/blob/main/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMapping.java#L169
@Override
protected HandlerMethod handleNoMatch(Set<RequestMappingInfo> infos, ...) throws Exception {
PartialMatchHelper helper = new PartialMatchHelper(infos, exchange);
if (helper.isEmpty()) {
return null;
}
...
}
And this method may allow an empty value in Set<RequestMappingInfo> infos parameter.
(Unfortunately, it happens a lot in the service I am running.)
The constructor code of org.springframework.web.reactive.result.method.RequestMappingInfoHandlerMapping.PartialMatchHelper is shown below. https://github.com/spring-projects/spring-framework/blob/main/spring-webflux/src/main/java/org/springframework/web/reactive/result/method/RequestMappingInfoHandlerMapping.java#L227
public PartialMatchHelper(Set<RequestMappingInfo> infos, ServerWebExchange exchange) {
this.partialMatches.addAll(infos.stream().
filter(info -> info.getPatternsCondition().getMatchingCondition(exchange) != null).
map(info -> new PartialMatch(info, exchange)).
collect(Collectors.toList()));
}
This code executes follwoing method even if there is no value in infos
java.util.Collection#stream
java.util.stream.Collectors#toList
java.util.stream.ReferencePipeline#collect(java.util.stream.Collector<? super P_OUT,A,R>)
Unnecessary resources are used via these methods.(create objects and execute operations, so )
Even though parameters have values, it seems better not to use Stream.
It can save resources by changing the constructor like mvc's RequestMappingInfoHandlerMapping. https://github.com/spring-projects/spring-framework/blob/main/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfoHandlerMapping.java#L300
public PartialMatchHelper(Set<RequestMappingInfo> infos, HttpServletRequest request) {
for (RequestMappingInfo info : infos) {
if (info.getActivePatternsCondition().getMatchingCondition(request) != null) {
this.partialMatches.add(new PartialMatch(info, request));
}
}
}
Below is a test created similar to the code above. 1) style of webflux 2) style of mvc 3) style of reuse object
- test code
public class PartialMatchHelperTest {
@Test
public void partialMatchHelperTest() {
int count = 1000 * 1000 * 100;
ThreadMXBean mxBean = ManagementFactory.getThreadMXBean();
// 1)
long startTime = System.currentTimeMillis();
final long startCpuTime = mxBean.getCurrentThreadCpuTime();
for (int i = 0; i < count; i++) {
new PartialMatchHelper(Collections.emptySet());
}
System.out.println(System.currentTimeMillis() - startTime);
System.out.println(mxBean.getCurrentThreadCpuTime() - startCpuTime);
// 2)
long startTime2 = System.currentTimeMillis();
final long startCpuTime2 = mxBean.getCurrentThreadCpuTime();
for (int i = 0; i < count; i++) {
new PartialMatchHelper2(Collections.emptySet());
}
System.out.println(System.currentTimeMillis() - startTime2);
System.out.println(mxBean.getCurrentThreadCpuTime() - startCpuTime2);
// 3)
long startTime3 = System.currentTimeMillis();
final long startCpuTime3 = mxBean.getCurrentThreadCpuTime();
for (int i = 0; i < count; i++) {
PartialMatchHelper3.of(Collections.emptySet());
}
System.out.println(System.currentTimeMillis() - startTime3);
System.out.println(mxBean.getCurrentThreadCpuTime() - startCpuTime3);
}
private static class PartialMatchHelper {
private final List<PartialMatch> partialMatches = new ArrayList<>();
public PartialMatchHelper(Set<RequestMappingInfo> infos) {
partialMatches.addAll(infos.stream().
filter(info -> info.getPatternsCondition().getMatchingCondition(null) != null).
map(info -> new PartialMatch(info)).
collect(Collectors.toList()));
}
public boolean isEmpty() {
return partialMatches.isEmpty();
}
}
private class PartialMatchHelper2 {
private final List<PartialMatch> partialMatches = new ArrayList<>();
public PartialMatchHelper2(Set<RequestMappingInfo> infos) {
for (RequestMappingInfo info : infos) {
if (info.getMatchingCondition(null) != null) {
this.partialMatches.add(new PartialMatch(info));
}
}
}
public boolean isEmpty() {
return partialMatches.isEmpty();
}
}
private static class PartialMatchHelper3 {
static PartialMatchHelper3 DISALBELD = new PartialMatchHelper3(Collections.emptySet());
private final List<PartialMatch> partialMatches = new ArrayList<>();
public static PartialMatchHelper3 of(Set<RequestMappingInfo> infos) {
if (infos.isEmpty()) {
return DISALBELD;
} else {
return new PartialMatchHelper3(infos);
}
}
public PartialMatchHelper3(Set<RequestMappingInfo> infos) {
for (RequestMappingInfo info : infos) {
if (info.getMatchingCondition(null) != null) {
this.partialMatches.add(new PartialMatch(info));
}
}
}
public boolean isEmpty() {
return partialMatches.isEmpty();
}
}
private static class PartialMatch {
private final RequestMappingInfo info;
public PartialMatch(RequestMappingInfo info) {
this.info = info;
}
public RequestMappingInfo getInfo() {
return this.info;
}
@Override
public String toString() {
return this.info.toString();
}
}
}
Result (from Async Profiler)
1) samples : CPU 393, Memory allocations 13,474,672
2) samples : CPU 580, Memory allocations 4,804,133,448
3) samples : CPU 6,547, Memory allocations 37,581,912,072
In my opinion, the following order seems like a good way. (Suggested solution (3 + 2) -> 3 -> 2 -> 1)
I will make PR according to your opinion.
Thanks :)
Comment From: koo-taejin
I have worked for Only the initialization part mentioned in the issue. The performance of Stream itself is not good, so it would be better if other parts were improved. http://www.angelikalanger.com/Conferences/Videos/Conference-Video-GeeCon-2015-Performance-Model-of-Streams-in-Java-8-Angelika-Langer.html
If you want to change the other parts to the collector's loop as before. Please let me know, then I am going to work for it.
Thanks :)
Comment From: rstoyanchev
Backported to 5.3.25 with #29667.