Several benchmarks underlined a few hotspots for CPU and GC pressure in the Spring Framework codebase:

  • org.springframework.util.MimeType.<init>(String, String, Map)
  • org.springframework.util.LinkedCaseInsensitiveMap.convertKey(String)

Both are linked with HTTP request headers parsing and manipulation during the request processing phase.

The first one is linked to repeated calls to HttpHeaders.getContentType within a single request handling. The media type parsing operation is expensive and the result doesn't change between calls, since the request headers are immutable at that point.

This commit improves this by caching the parsed MediaType for the "Content-Type" request header in the ReadOnlyHttpHeaders class. This change is available for both Spring MVC and Spring WebFlux.

The second one is linked to insertions/lookups in the LinkedCaseInsensitiveMap, which is the data structure behind HttpHeaders. Those operations are creating a lot of garbage (including a lot of String created by toLowerCase). We could choose a more efficient data structure for storing HTTP headers data.

As a first step, this commit is focusing on Spring WebFlux and introduces MultiValueMap implementations mapped by native HTTP headers for the following servers: Tomcat, Jetty, Reactor Netty and Undertow. Such implementations avoid unnecessary copying of the request headers and leverages as much as possible optimized operations provided by the native implementations.

This change has a few consequences:

  • HttpHeaders can now wrap a MultiValueMap directly
  • The default constructor of HttpHeaders is still backed by a LinkedCaseInsensitiveMap
  • The HTTP request headers for the websocket HTTP handshake now need to be cloned, because native headers are likely to be pooled/recycled by the server implementation, hence gone when the initial HTTP exchange is done

Issue: SPR-17250

Comment From: bclozel

Hey @jhoeller , @rstoyanchev

As promised, here's a first review cycle for this PR.

Caching the parsed content-type is done for both Spring WebFlux and Spring MVC, but the native headers optimization is not applied to Spring MVC because 1) this might not be the main performance issue in Spring MVC right now and 2) there's only so much we can do with the Servlet API as a common bassis. We can revisit that later if we need to.

For now, this PR is trying to stick as much as possible to the previous behavior. I've added MultiValueMap implementations that wrap the native headers. Those are often then wrapped by an immutable MultiValueMap implementation when the request is declared as read only.

I'm currently running local benchmarks to measure the difference in CPU + allocation overhead created by Spring. I'll get back to you on that soon.

Possible improvements

We could go one step further and make those wrappers immutable from the start, but this clashes with the general architecture and might not work in certain places.

The content-type optimization only kicks in for HTTP PUT/POST requests where we actually look at the request content-type. The second optimization only applies to Spring WebFlux. We could improve the Spring MVC situation a bit by using a different implementation than LinkedCaseInsensitiveMap. My benchmarks show that a simple List + String.equalsIgnoreCase lookups or even a TreeMap + case insensitive hashcode yield better results, since toLowerCase is the expensive part here (creating String and the complex logic with Locale).

Let me know what you think!

Comment From: bclozel

I've applied @poutsma 's comments (and we fixed some more things as well). I've just applied the same reasoning to the HTTP response headers and made sure to avoid HTTP headers copying in DefaultServerHttpRequestBuilder - those changes were applied in two new commits.

Comment From: rstoyanchev

Sorry but it looks I left some of my comments on the original commit https://github.com/bclozel/spring-framework/commit/320433bb1e0ecf9d6fd38d9994ce453220956aa1#diff-9b3604016549e725bd1a952499ba5c6c and they do not show up here..

Overall this looks alright.

One general concern I have is with Tomcat and Jetty where basic operations like get, put, remove, create a new List per invocation. I get why Tomcat and Jetty don't use a Map internally but us having to adapt to List<String> makes things worse. Looking at our HttpHeaders, a lot of the typed get methods actually returns lists of things. Perhaps we should consider adding more caching in the read-only wrapper in order to offset this side effect.

The second comment is the possibility of using the Netty and Jetty header adapters on the client response side seems a straight-forward fit, if it wasn't for the fact there is no convenient package for sharing things between client and server (aside from the top-level http package).

Comment From: rstoyanchev

One more comment. In AbstractServerHttpResponse the getHeaders method returns a new read-only wrapper on every call after committed. Perhaps we can save the read-only wrapper and create it once?