From 351bfa5585367fab8bd9c796fcc6bd6d122c60bd Mon Sep 17 00:00:00 2001 From: gregw Date: Tue, 3 Nov 2020 14:28:51 +0100 Subject: [PATCH 1/6] 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. --- .../org/eclipse/jetty/http/HttpParser.java | 69 +++++++++++++++---- 1 file changed, 55 insertions(+), 14 deletions(-) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java index 9cad979abd44..29f32e186a9c 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java @@ -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; @@ -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 NO_CACHE = Collections.emptyList(); /** * Cache of common {@link HttpField}s including: */ public static final Trie CACHE = new ArrayTrie<>(2048); - private static final Trie NO_CACHE = AbstractTrie.emptyTrie(true); + private static final Trie NO_CACHE = Trie.empty(true); // States public enum FieldState @@ -1037,7 +1036,9 @@ else if (_endOfContent == EndOfContent.CHUNKED_CONTENT) case CONNECTION: // Don't cache headers if not persistent - if (_handler.getHeaderCacheSize() > 0 && (HttpHeaderValue.CLOSE.is(_valueString) || _valueString.contains(HttpHeaderValue.CLOSE.asString()))) + 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; @@ -1061,7 +1062,7 @@ else if (_endOfContent == EndOfContent.CHUNKED_CONTENT) { if (_fieldCache == null) { - _fieldCache = (_handler.getHeaderCacheSize() > 0 && (_version != null && _version.getVersion() == HttpVersion.HTTP_1_1.getVersion())) + _fieldCache = (_handler.getHeaderCacheSize() > 0 && (_version != null && _version == HttpVersion.HTTP_1_1)) ? new ArrayTernaryTrie<>(_handler.getHeaderCacheSize()) : NO_CACHE; } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/AbstractTrie.java b/jetty-util/src/main/java/org/eclipse/jetty/util/AbstractTrie.java index 4c4c7d254059..cffde1316386 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/AbstractTrie.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/AbstractTrie.java @@ -20,7 +20,6 @@ import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; -import java.util.Set; /** * Abstract Trie implementation. @@ -82,57 +81,4 @@ public boolean isCaseInsensitive() { return _caseInsensitive; } - - public static Trie emptyTrie(boolean caseInsensitive) - { - return new AbstractTrie(caseInsensitive) - { - @Override - public boolean put(String s, T t) - { - return false; - } - - @Override - public T get(String s, int offset, int len) - { - return null; - } - - @Override - public T get(ByteBuffer b, int offset, int len) - { - return null; - } - - @Override - public T getBest(String s, int offset, int len) - { - return null; - } - - @Override - public T getBest(ByteBuffer b, int offset, int len) - { - return null; - } - - @Override - public Set keySet() - { - return null; - } - - @Override - public boolean isFull() - { - return true; - } - - @Override - public void clear() - { - } - }; - } } diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/Trie.java b/jetty-util/src/main/java/org/eclipse/jetty/util/Trie.java index 31536d9f67ad..77cba241f6ee 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/Trie.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/Trie.java @@ -131,4 +131,99 @@ public interface Trie boolean isCaseInsensitive(); void clear(); + + static Trie empty(final boolean caseInsensitive) + { + return new Trie() + { + @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 keySet() + { + return null; + } + + @Override + public boolean isFull() + { + return true; + } + + @Override + public boolean isCaseInsensitive() + { + return caseInsensitive; + } + + @Override + public void clear() + { + } + }; + } } From 1ea412c4a47c16d3acbde7289c3a8c6133b33390 Mon Sep 17 00:00:00 2001 From: gregw Date: Wed, 11 Nov 2020 18:36:39 +0100 Subject: [PATCH 6/6] Update from review + more javadoc + empty set return --- .../src/main/java/org/eclipse/jetty/http/HttpHeader.java | 3 +++ .../java/org/eclipse/jetty/server/HttpConfiguration.java | 5 +++-- jetty-util/src/main/java/org/eclipse/jetty/util/Trie.java | 3 ++- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpHeader.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpHeader.java index de5aeec74e79..11fedafa87d9 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpHeader.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpHeader.java @@ -195,6 +195,9 @@ 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; diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConfiguration.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConfiguration.java index e0841f256e75..883b103c28ec 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConfiguration.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConfiguration.java @@ -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; @@ -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) { diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/Trie.java b/jetty-util/src/main/java/org/eclipse/jetty/util/Trie.java index 77cba241f6ee..258e6fd7f57d 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/Trie.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/Trie.java @@ -19,6 +19,7 @@ package org.eclipse.jetty.util; import java.nio.ByteBuffer; +import java.util.Collections; import java.util.Set; /** @@ -205,7 +206,7 @@ public T getBest(ByteBuffer b, int offset, int len) @Override public Set keySet() { - return null; + return Collections.emptySet(); } @Override