Long URL causes high memory allocation in `WebClient` calls

[jeffreye]

Hi team,

We are sending GET request with long urls (often longer than 200 characters due to various parameters)

Recently we tried to migrate it from apache http client to Webclient but found a performance hotspot in

spring-framework/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/WebClientUtils.java

Lines 75 to 83 in 2ee4c3a

	public static String getRequestDescription(HttpMethod httpMethod, URI uri) {
	if (StringUtils.hasText(uri.getQuery())) {
	try {
	uri = new URI(uri.getScheme(), null, uri.getHost(), uri.getPort(), uri.getPath(), null, null);
	}
	catch (URISyntaxException ignored) {
	}
	}
	return httpMethod.name() + " " + uri;

This method is primarily used for debugging / logging

spring-framework/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultWebClient.java

Lines 464 to 466 in 2ee4c3a

	.checkpoint("Request to " +
	WebClientUtils.getRequestDescription(request.method(), request.url()) +
	" [DefaultWebClient]")

Could you help make a fix on them?

[bclozel]

Which Spring Framework version are you using?
Can you share the result of your performance tool, is there a particular method contributing to this?
You are mentioning long URLs with (I assume) lots of query parameters, but we intentionally skip the query parameters there so this shouldn't contribute.

[MintBee]

I tried it on 7.1.0-SNAPSHOT.
The cause is getRequestDescription(), called on every http call, is creating new URI object every single time if there is a query stirng.

Creation of URI object is quite expensive since it performs validation and parsing for the constructor input, which is already done by the URI argument.

I tried JMH testing for it. It did not fully eliminated performance difference between WebClient and apache client, but improving this would surely eliminate the gap between long query string and short query string.

JMH gist I used for testing - rationale included

I made change to close the issue too. Added regression test first, then applied changes. commit

[MintBee]

By the way, WebClientUtils.getRequestDescription() strips out userInfo and fragment only if there is query string. userInfo and fragment remain when there is no queryString. I persisted this behavior for the commit, but not sure it's intended.

spring-framework/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/WebClientUtils.java

Lines 75 to 83 in 2ee4c3a

	public static String getRequestDescription(HttpMethod httpMethod, URI uri) {
	if (StringUtils.hasText(uri.getQuery())) {
	try {
	uri = new URI(uri.getScheme(), null, uri.getHost(), uri.getPort(), uri.getPath(), null, null);
	}
	catch (URISyntaxException ignored) {
	}
	}
	return httpMethod.name() + " " + uri;

You can see userInfo, queryString, and fragment are all nullified for the URI creation.

[bclozel]

@MintBee are you working with @jeffreye ? Can you share the performance results they mentioned?

I am aware that creating a URI instance is not free but I would assume that it's not so important that it shows on a typical flame graph on an application request. The original report also mentions memory allocation so I'm not sure what the report is really about at this point.

[jeffreye]

Thanks @MintBee , you helped me a lot.
@bclozel those extra objects and strings creation caused > 5% performance degradation for our application so we won't be able to finish the migration.
I'm mentioning allocation because I saw we are having too much string creation for debugging purpose (e.g. checkpoint description could be more simplified)