Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix #5562 Improve HTTP Field cache allocation #5565

Merged
merged 9 commits into from Nov 12, 2020
Merged

Fix #5562 Improve HTTP Field cache allocation #5565

Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
351bfa5
Fix #5562 Improve HTTP Field cache allocation
gregw Nov 3, 2020
48f5abd
Merge remote-tracking branch 'origin/jetty-9.4.x' into jetty-9.4.x-55…
gregw Nov 3, 2020
2a3c589
Merge remote-tracking branch 'origin/jetty-9.4.x' into jetty-9.4.x-55…
gregw Nov 4, 2020
70542dd
Fixed NPE
gregw Nov 4, 2020
5660e28
Feedback from review
gregw Nov 4, 2020
339c86d
Feedback from review
gregw Nov 4, 2020
a8396f4
Updates from review
gregw Nov 5, 2020
7c2cd2c
Merge branch 'jetty-9.4.x' into jetty-9.4.x-5562-improve-field-cache
gregw Nov 11, 2020
1ea412c
Update from review
gregw Nov 11, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
69 changes: 55 additions & 14 deletions jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java
View file
Open in desktop
Expand Up @@ -20,6 +20,8 @@

import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Collections;
import java.util.EnumSet;
import java.util.List;
import java.util.Locale;
Expand Down Expand Up @@ -89,6 +91,7 @@ public class HttpParser
public static final String __STRICT = "org.eclipse.jetty.http.HttpParser.STRICT";
public static final int INITIAL_URI_LENGTH = 256;
private static final int MAX_CHUNK_LENGTH = Integer.MAX_VALUE / 16 - 16;
private static final List<HttpField> NO_CACHE = Collections.emptyList();

/**
* Cache of common {@link HttpField}s including: <UL>
Expand Down Expand Up @@ -152,6 +155,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 +171,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 All @@ -179,6 +182,7 @@ public enum State
private boolean _cr;
private ByteBuffer _contentChunk;
private Trie<HttpField> _fieldCache;
private List<HttpField> _cacheableFields;

private int _length;
private final StringBuilder _string = new StringBuilder();
Expand Down Expand Up @@ -231,7 +235,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.asString().startsWith(":") && !CACHE.put(new HttpField(h, (String)null)))
gregw marked this conversation as resolved.
Show resolved Hide resolved
throw new IllegalStateException("CACHE FULL");
}
}
Expand Down Expand Up @@ -884,10 +888,8 @@ 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);
// Should we cache header fields?
getFieldCache();
gregw marked this conversation as resolved.
Show resolved Hide resolved

setState(State.HEADER);

Expand Down Expand Up @@ -961,7 +963,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 +1036,18 @@ 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))
if (_handler.getHeaderCacheSize() > 0 &&
(HttpHeaderValue.CLOSE.is(_valueString) || _valueString.contains(HttpHeaderValue.CLOSE.asString())))
{
_cacheableFields = NO_CACHE;
_fieldCache = null;
}
break;

case AUTHORIZATION:
Expand All @@ -1052,18 +1058,42 @@ 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)
{
// If a non full cache already exists, then we can add to it.
if (!_fieldCache.isFull())
{
if (_field == null)
_field = new HttpField(_header, caseInsensitiveHeader(_headerString, _header.asString()), _valueString);
_fieldCache.put(_field);
}
}
else if (_cacheableFields != NO_CACHE)
{
if (_handler.getHeaderCacheSize() <= 0 || (_version != null && _version.getVersion() != HttpVersion.HTTP_1_1.getVersion()))
// Don't cache any fields
_cacheableFields = NO_CACHE;
gregw marked this conversation as resolved.
Show resolved Hide resolved
else
{
// This must be the first request seen by this parser, so just create a simple list of
// headers that can later to converted to _fieldCache if another request comes.
if (_cacheableFields == null)
_cacheableFields = new ArrayList<>();
if (_field == null)
_field = new HttpField(_header, caseInsensitiveHeader(_headerString, _header.asString()), _valueString);
_cacheableFields.add(_field);
gregw marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
}
_handler.parsedHeader(_field != null ? _field : new HttpField(_header, _headerString, _valueString));
Expand Down Expand Up @@ -1901,6 +1931,17 @@ protected void setState(FieldState state)

public Trie<HttpField> getFieldCache()
{
// if we have cacheableFields from a previous request, we can make a cache
if (_fieldCache == null && _cacheableFields != null && _cacheableFields != NO_CACHE)
{
_fieldCache = new ArrayTernaryTrie<>(_handler.getHeaderCacheSize());
for (HttpField field : _cacheableFields)
{
if (_fieldCache.isFull() || !_fieldCache.put(field))
break;
}
_cacheableFields = null;
}
return _fieldCache;
}

Expand Down