Skip to content

Commit

Permalink
Optimize HTTP headers management
Browse files Browse the repository at this point in the history
Several benchmarks underlined a few hotspots for CPU and GC pressure in
the Spring Framework codebase:

1. `org.springframework.util.MimeType.<init>(String, String, Map)`
2. `org.springframework.util.LinkedCaseInsensitiveMap.convertKey(String)`

Both are linked with HTTP request headers parsing and response headers
writin during the exchange processing phase.

1) 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.

2) 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, Netty and Undertow.
Such implementations avoid unnecessary copying of the 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
  • Loading branch information
bclozel committed Oct 11, 2018
1 parent 61403e3 commit ce7278a
Show file tree
Hide file tree
Showing 19 changed files with 1,224 additions and 85 deletions.
61 changes: 26 additions & 35 deletions spring-web/src/main/java/org/springframework/http/HttpHeaders.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,6 @@
import java.util.Collections;
import java.util.EnumSet;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.LinkedList;
import java.util.List;
import java.util.Locale;
import java.util.Map;
Expand All @@ -47,7 +45,9 @@

import org.springframework.lang.Nullable;
import org.springframework.util.Assert;
import org.springframework.util.CollectionUtils;
import org.springframework.util.LinkedCaseInsensitiveMap;
import org.springframework.util.LinkedMultiValueMap;
import org.springframework.util.MultiValueMap;
import org.springframework.util.StringUtils;

Expand Down Expand Up @@ -78,7 +78,8 @@ public class HttpHeaders implements MultiValueMap<String, String>, Serializable
/**
* The empty {@code HttpHeaders} instance (immutable).
*/
public static final HttpHeaders EMPTY = new HttpHeaders(new LinkedHashMap<>(), true);
public static final HttpHeaders EMPTY =
new ReadOnlyHttpHeaders(new HttpHeaders(new LinkedMultiValueMap<>(0)));
/**
* The HTTP {@code Accept} header field name.
* @see <a href="http://tools.ietf.org/html/rfc7231#section-5.3.2">Section 5.3.2 of RFC 7231</a>
Expand Down Expand Up @@ -397,35 +398,27 @@ public class HttpHeaders implements MultiValueMap<String, String>, Serializable
private static final DateTimeFormatter[] DATE_FORMATTERS = new DateTimeFormatter[] {
DateTimeFormatter.RFC_1123_DATE_TIME,
DateTimeFormatter.ofPattern("EEEE, dd-MMM-yy HH:mm:ss zz", Locale.US),
DateTimeFormatter.ofPattern("EEE MMM dd HH:mm:ss yyyy",Locale.US).withZone(GMT)
DateTimeFormatter.ofPattern("EEE MMM dd HH:mm:ss yyyy", Locale.US).withZone(GMT)
};


private final Map<String, List<String>> headers;

private final boolean readOnly;
final MultiValueMap<String, String> headers;


/**
* Constructs a new, empty instance of the {@code HttpHeaders} object.
* Construct a new, empty instance of the {@code HttpHeaders} object.
*/
public HttpHeaders() {
this(new LinkedCaseInsensitiveMap<>(8, Locale.ENGLISH), false);
this(CollectionUtils.toMultiValueMap(
new LinkedCaseInsensitiveMap<>(8, Locale.ENGLISH)));
}

/**
* Private constructor that can create read-only {@code HttpHeader} instances.
* Construct a new {@code HttpHeaders} instance backed by an existing map.
*/
private HttpHeaders(Map<String, List<String>> headers, boolean readOnly) {
if (readOnly) {
Map<String, List<String>> map = new LinkedCaseInsensitiveMap<>(headers.size(), Locale.ENGLISH);
headers.forEach((key, valueList) -> map.put(key, Collections.unmodifiableList(valueList)));
this.headers = Collections.unmodifiableMap(map);
}
else {
this.headers = headers;
}
this.readOnly = readOnly;
public HttpHeaders(MultiValueMap<String, String> headers) {
Assert.notNull(headers, "headers must not be null");
this.headers = headers;
}


Expand Down Expand Up @@ -1474,8 +1467,7 @@ protected String toCommaDelimitedString(List<String> headerValues) {
@Override
@Nullable
public String getFirst(String headerName) {
List<String> headerValues = this.headers.get(headerName);
return (headerValues != null ? headerValues.get(0) : null);
return this.headers.getFirst(headerName);
}

/**
Expand All @@ -1488,19 +1480,17 @@ public String getFirst(String headerName) {
*/
@Override
public void add(String headerName, @Nullable String headerValue) {
List<String> headerValues = this.headers.computeIfAbsent(headerName, k -> new LinkedList<>());
headerValues.add(headerValue);
this.headers.add(headerName, headerValue);
}

@Override
public void addAll(String key, List<? extends String> values) {
List<String> currentValues = this.headers.computeIfAbsent(key, k -> new LinkedList<>());
currentValues.addAll(values);
this.headers.addAll(key, values);
}

@Override
public void addAll(MultiValueMap<String, String> values) {
values.forEach(this::addAll);
this.headers.addAll(values);
}

/**
Expand All @@ -1513,21 +1503,17 @@ public void addAll(MultiValueMap<String, String> values) {
*/
@Override
public void set(String headerName, @Nullable String headerValue) {
List<String> headerValues = new LinkedList<>();
headerValues.add(headerValue);
this.headers.put(headerName, headerValues);
this.headers.set(headerName, headerValue);
}

@Override
public void setAll(Map<String, String> values) {
values.forEach(this::set);
this.headers.setAll(values);
}

@Override
public Map<String, String> toSingleValueMap() {
LinkedHashMap<String, String> singleValueMap = new LinkedHashMap<>(this.headers.size());
this.headers.forEach((key, valueList) -> singleValueMap.put(key, valueList.get(0)));
return singleValueMap;
return this.headers.toSingleValueMap();
}


Expand Down Expand Up @@ -1623,7 +1609,12 @@ public String toString() {
*/
public static HttpHeaders readOnlyHttpHeaders(HttpHeaders headers) {
Assert.notNull(headers, "HttpHeaders must not be null");
return (headers.readOnly ? headers : new HttpHeaders(headers, true));
if (headers instanceof ReadOnlyHttpHeaders) {
return headers;
}
else {
return new ReadOnlyHttpHeaders(headers);
}
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
/*
* Copyright 2002-2018 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.springframework.http;

import java.util.AbstractMap;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;

import org.springframework.lang.Nullable;
import org.springframework.util.MultiValueMap;

/**
* {@code HttpHeaders} object that can only be read, not written to.
*
* @author Brian Clozel
* @since 5.1
*/
class ReadOnlyHttpHeaders extends HttpHeaders {

private static final long serialVersionUID = -8578554704772377436L;

@Nullable
private MediaType cachedContentType;

ReadOnlyHttpHeaders(HttpHeaders headers) {
super(headers.headers);
}

@Override
public MediaType getContentType() {
if (this.cachedContentType != null) {
return this.cachedContentType;
}
else {
MediaType contentType = super.getContentType();
this.cachedContentType = contentType;
return contentType;
}
}

@Override
public List<String> get(Object key) {
List<String> values = this.headers.get(key);
if (values != null) {
return Collections.unmodifiableList(values);
}
return values;
}

@Override
public void add(String headerName, @Nullable String headerValue) {
throw new UnsupportedOperationException();
}

@Override
public void addAll(String key, List<? extends String> values) {
throw new UnsupportedOperationException();
}

@Override
public void addAll(MultiValueMap<String, String> values) {
throw new UnsupportedOperationException();
}

@Override
public void set(String headerName, @Nullable String headerValue) {
throw new UnsupportedOperationException();
}

@Override
public void setAll(Map<String, String> values) {
throw new UnsupportedOperationException();
}

@Override
public Map<String, String> toSingleValueMap() {
return Collections.unmodifiableMap(this.headers.toSingleValueMap());
}

@Override
public Set<String> keySet() {
return Collections.unmodifiableSet(this.headers.keySet());
}

@Override
public List<String> put(String key, List<String> value) {
throw new UnsupportedOperationException();
}

@Override
public List<String> remove(Object key) {
throw new UnsupportedOperationException();
}

@Override
public void putAll(Map<? extends String, ? extends List<String>> map) {
throw new UnsupportedOperationException();
}

@Override
public void clear() {
throw new UnsupportedOperationException();
}

@Override
public Collection<List<String>> values() {
return Collections.unmodifiableCollection(this.headers.values());
}

@Override
public Set<Entry<String, List<String>>> entrySet() {
return Collections.unmodifiableSet(this.headers.entrySet().stream()
.map(AbstractMap.SimpleImmutableEntry::new)
.collect(Collectors.toSet()));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,9 @@ public List<String> get(Object key) {
Assert.isInstanceOf(String.class, key, "Key must be a String-based header name");

Collection<String> values1 = servletResponse.getHeaders((String) key);
if (headersWritten) {
return new ArrayList<>(values1);
}
boolean isEmpty1 = CollectionUtils.isEmpty(values1);

List<String> values2 = super.get(key);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2016 the original author or authors.
* Copyright 2002-2018 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -24,6 +24,7 @@

import org.springframework.core.io.buffer.DataBuffer;
import org.springframework.core.io.buffer.DataBufferFactory;
import org.springframework.http.HttpHeaders;

/**
* Abstract base class for listener-based server responses, e.g. Servlet 3.1
Expand All @@ -41,6 +42,10 @@ public AbstractListenerServerHttpResponse(DataBufferFactory dataBufferFactory) {
super(dataBufferFactory);
}

public AbstractListenerServerHttpResponse(DataBufferFactory dataBufferFactory, HttpHeaders headers) {
super(dataBufferFactory, headers);
}


@Override
protected final Mono<Void> writeWithInternal(Publisher<? extends DataBuffer> body) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,14 @@ private enum State {NEW, COMMITTING, COMMITTED}


public AbstractServerHttpResponse(DataBufferFactory dataBufferFactory) {
this(dataBufferFactory, new HttpHeaders());
}

public AbstractServerHttpResponse(DataBufferFactory dataBufferFactory, HttpHeaders headers) {
Assert.notNull(dataBufferFactory, "DataBufferFactory must not be null");
Assert.notNull(headers, "HttpHeaders must not be null");
this.dataBufferFactory = dataBufferFactory;
this.headers = new HttpHeaders();
this.headers = headers;
this.cookies = new LinkedMultiValueMap<>();
}

Expand Down
Loading

0 comments on commit ce7278a

Please sign in to comment.