Skip to content

Commit

Permalink
Fix #5562 Improve HTTP Field cache allocation (#5565)
Browse files Browse the repository at this point in the history 
* Fix #5562 Improve HTTP Field cache allocation

Fix #5562 by initially putting cacheable fields into a inexpensive arraylist.
Only create the Trie (with space and complexity costs) if a second request is received.

* Fixed NPE

* Feedback from review

Create `HttpHeader.isPseudo()`` method
improved clarity with `createFieldCacheIfNeeded()``

* Feedback from review

Only defer Trie creation to first cacheable field, not until next request.

* Updates from review

* Update from review

 + more javadoc
 + empty set return
  • Loading branch information
@gregw
gregw committed Nov 12, 2020
1 parent 3660e38 commit f4c32e7
Show file tree
Hide file tree
Showing 4 changed files with 145 additions and 24 deletions.
27 changes: 21 additions & 6 deletions jetty-http/src/main/java/org/eclipse/jetty/http/HttpHeader.java
View file
Expand Up @@ -128,13 +128,13 @@ public enum HttpHeader
/**
* HTTP2 Fields.
*/
C_METHOD(":method"),
C_SCHEME(":scheme"),
C_AUTHORITY(":authority"),
C_PATH(":path"),
C_STATUS(":status"),
C_METHOD(":method", true),
C_SCHEME(":scheme", true),
C_AUTHORITY(":authority", true),
C_PATH(":path", true),
C_STATUS(":status", true),

UNKNOWN("::UNKNOWN::");
UNKNOWN("::UNKNOWN::", true);

public static final Trie<HttpHeader> CACHE = new ArrayTrie<>(630);

Expand All @@ -153,14 +153,21 @@ public enum HttpHeader
private final byte[] _bytes;
private final byte[] _bytesColonSpace;
private final ByteBuffer _buffer;
private final boolean _pseudo;

HttpHeader(String s)
{
this(s, false);
}

HttpHeader(String s, boolean pseudo)
{
_string = s;
_lowerCase = StringUtil.asciiToLowerCase(s);
_bytes = StringUtil.getBytes(s);
_bytesColonSpace = StringUtil.getBytes(s + ": ");
_buffer = ByteBuffer.wrap(_bytes);
_pseudo = pseudo;
}

public String lowerCaseName()
Expand Down Expand Up @@ -188,6 +195,14 @@ public boolean is(String s)
return _string.equalsIgnoreCase(s);
}

/**
* @return True if the header is a HTTP2 Pseudo header (eg ':path')
*/
public boolean isPseudo()
{
return _pseudo;
}

public String asString()
{
return _string;
Expand Down
41 changes: 25 additions & 16 deletions jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java
View file
Expand Up @@ -103,6 +103,7 @@ public class HttpParser
* </ul>
*/
public static final Trie<HttpField> CACHE = new ArrayTrie<>(2048);
private static final Trie<HttpField> NO_CACHE = Trie.empty(true);

// States
public enum FieldState
Expand Down Expand Up @@ -152,6 +153,7 @@ public enum State
private final int _maxHeaderBytes;
private final HttpCompliance _compliance;
private final EnumSet<HttpComplianceSection> _compliances;
private final Utf8StringBuilder _uri = new Utf8StringBuilder(INITIAL_URI_LENGTH);
private HttpField _field;
private HttpHeader _header;
private String _headerString;
Expand All @@ -167,7 +169,6 @@ public enum State
private HttpMethod _method;
private String _methodString;
private HttpVersion _version;
private Utf8StringBuilder _uri = new Utf8StringBuilder(INITIAL_URI_LENGTH); // Tune?
private EndOfContent _endOfContent;
private boolean _hasContentLength;
private boolean _hasTransferEncoding;
Expand Down Expand Up @@ -231,7 +232,7 @@ public enum State
// Add headers with null values so HttpParser can avoid looking up name again for unknown values
for (HttpHeader h : HttpHeader.values())
{
if (!CACHE.put(new HttpField(h, (String)null)))
if (!h.isPseudo() && !CACHE.put(new HttpField(h, (String)null)))
throw new IllegalStateException("CACHE FULL");
}
}
Expand Down Expand Up @@ -884,11 +885,6 @@ else if (n == HttpTokens.LINE_FEED)
}
checkVersion();

// Should we try to cache header fields?
int headerCache = _handler.getHeaderCacheSize();
if (_fieldCache == null && _version.getVersion() >= HttpVersion.HTTP_1_1.getVersion() && headerCache > 0)
_fieldCache = new ArrayTernaryTrie<>(headerCache);

setState(State.HEADER);

_requestHandler.startRequest(_methodString, _uri.toString(), _version);
Expand Down Expand Up @@ -961,7 +957,7 @@ private void parsedHeader()
// Handle known headers
if (_header != null)
{
boolean addToConnectionTrie = false;
boolean addToFieldCache = false;
switch (_header)
{
case CONTENT_LENGTH:
Expand Down Expand Up @@ -1034,14 +1030,16 @@ else if (_endOfContent == EndOfContent.CHUNKED_CONTENT)
_field = new HostPortHttpField(_header,
_compliances.contains(HttpComplianceSection.FIELD_NAME_CASE_INSENSITIVE) ? _header.asString() : _headerString,
_valueString);
addToConnectionTrie = _fieldCache != null;
addToFieldCache = true;
}
break;

case CONNECTION:
// Don't cache headers if not persistent
if (HttpHeaderValue.CLOSE.is(_valueString) || new QuotedCSV(_valueString).getValues().stream().anyMatch(HttpHeaderValue.CLOSE::is))
_fieldCache = null;
if (_field == null)
_field = new HttpField(_header, caseInsensitiveHeader(_headerString, _header.asString()), _valueString);
if (_handler.getHeaderCacheSize() > 0 && _field.contains(HttpHeaderValue.CLOSE.asString()))
_fieldCache = NO_CACHE;
break;

case AUTHORIZATION:
Expand All @@ -1052,18 +1050,29 @@ else if (_endOfContent == EndOfContent.CHUNKED_CONTENT)
case COOKIE:
case CACHE_CONTROL:
case USER_AGENT:
addToConnectionTrie = _fieldCache != null && _field == null;
addToFieldCache = _field == null;
break;

default:
break;
}

if (addToConnectionTrie && !_fieldCache.isFull() && _header != null && _valueString != null)
// Cache field?
if (addToFieldCache && _header != null && _valueString != null)
{
if (_field == null)
_field = new HttpField(_header, caseInsensitiveHeader(_headerString, _header.asString()), _valueString);
_fieldCache.put(_field);
if (_fieldCache == null)
{
_fieldCache = (_handler.getHeaderCacheSize() > 0 && (_version != null && _version == HttpVersion.HTTP_1_1))
? new ArrayTernaryTrie<>(_handler.getHeaderCacheSize())
: NO_CACHE;
}

if (!_fieldCache.isFull())
{
if (_field == null)
_field = new HttpField(_header, caseInsensitiveHeader(_headerString, _header.asString()), _valueString);
_fieldCache.put(_field);
}
}
}
_handler.parsedHeader(_field != null ? _field : new HttpField(_header, _headerString, _valueString));
Expand Down
5 changes: 3 additions & 2 deletions jetty-server/src/main/java/org/eclipse/jetty/server/HttpConfiguration.java
View file
Expand Up @@ -198,7 +198,7 @@ public int getResponseHeaderSize()
return _responseHeaderSize;
}

@ManagedAttribute("The maximum allowed size in bytes for an HTTP header field cache")
@ManagedAttribute("The maximum allowed size in Trie nodes for an HTTP header field cache")
public int getHeaderCacheSize()
{
return _headerCacheSize;
Expand Down Expand Up @@ -423,7 +423,8 @@ public void setResponseHeaderSize(int responseHeaderSize)
}

/**
* @param headerCacheSize The size in bytes of the header field cache.
* @param headerCacheSize The size of the header field cache, in terms of unique characters branches
* in the lookup {@link Trie} and associated data structures.
*/
public void setHeaderCacheSize(int headerCacheSize)
{
Expand Down
96 changes: 96 additions & 0 deletions jetty-util/src/main/java/org/eclipse/jetty/util/Trie.java
View file
Expand Up @@ -19,6 +19,7 @@
package org.eclipse.jetty.util;

import java.nio.ByteBuffer;
import java.util.Collections;
import java.util.Set;

/**
Expand Down Expand Up @@ -131,4 +132,99 @@ public interface Trie<V>
boolean isCaseInsensitive();

void clear();

static <T> Trie<T> empty(final boolean caseInsensitive)
{
return new Trie<T>()
{
@Override
public boolean put(String s, Object o)
{
return false;
}

@Override
public boolean put(Object o)
{
return false;
}

@Override
public T remove(String s)
{
return null;
}

@Override
public T get(String s)
{
return null;
}

@Override
public T get(String s, int offset, int len)
{
return null;
}

@Override
public T get(ByteBuffer b)
{
return null;
}

@Override
public T get(ByteBuffer b, int offset, int len)
{
return null;
}

@Override
public T getBest(String s)
{
return null;
}

@Override
public T getBest(String s, int offset, int len)
{
return null;
}

@Override
public T getBest(byte[] b, int offset, int len)
{
return null;
}

@Override
public T getBest(ByteBuffer b, int offset, int len)
{
return null;
}

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

@Override
public boolean isFull()
{
return true;
}

@Override
public boolean isCaseInsensitive()
{
return caseInsensitive;
}

@Override
public void clear()
{
}
};
}
}

0 comments on commit f4c32e7

Please sign in to comment.