Fix contains in HttpFields name set and prove random access to HttpFields via EnumMap not worth it. (#11846)
Fix #11811 with javadoc and benchmark
This commit is contained in:
parent
9546b3ab49
commit
95059356c9
|
@ -38,9 +38,16 @@ import java.util.stream.StreamSupport;
|
|||
/**
|
||||
* <p>An ordered collection of {@link HttpField}s that represent the HTTP headers
|
||||
* or HTTP trailers of an HTTP request or an HTTP response.</p>
|
||||
*
|
||||
* <p>{@link HttpFields} is immutable and typically used in server-side HTTP requests
|
||||
* and client-side HTTP responses, while {@link HttpFields.Mutable} is mutable and
|
||||
* typically used in server-side HTTP responses and client-side HTTP requests.</p>
|
||||
*
|
||||
* <p>Access is always more efficient using {@link HttpHeader} keys rather than {@link String} field names.</p>
|
||||
*
|
||||
* <p>The primary implementations of {@code HttpFields} have been optimized assuming few
|
||||
* lookup operations, thus typically if many {@link HttpField}s need to looked up, it may be
|
||||
* better to use an {@link Iterator} to find multiple fields in a single iteration.</p>
|
||||
*/
|
||||
public interface HttpFields extends Iterable<HttpField>, Supplier<HttpFields>
|
||||
{
|
||||
|
@ -350,10 +357,12 @@ public interface HttpFields extends Iterable<HttpField>, Supplier<HttpFields>
|
|||
/**
|
||||
* <p>Returns whether this instance contains the given field name.</p>
|
||||
* <p>The comparison of field name is case-insensitive via
|
||||
* {@link HttpField#is(String)}.
|
||||
* {@link HttpField#is(String)}. If possible, it is more efficient to use
|
||||
* {@link #contains(HttpHeader)}.
|
||||
*
|
||||
* @param name the case-insensitive field name to search for
|
||||
* @return whether this instance contains the given field name
|
||||
* @see #contains(HttpHeader)
|
||||
*/
|
||||
default boolean contains(String name)
|
||||
{
|
||||
|
@ -412,7 +421,7 @@ public interface HttpFields extends Iterable<HttpField>, Supplier<HttpFields>
|
|||
* <p>Returns the encoded value of the first field with the given field name,
|
||||
* or {@code null} if no such field is present.</p>
|
||||
* <p>The comparison of field name is case-insensitive via
|
||||
* {@link HttpField#is(String)}.</p>
|
||||
* {@link HttpField#is(String)}. If possible, it is more efficient to use {@link #get(HttpHeader)}.</p>
|
||||
* <p>In case of multi-valued fields, the returned value is the encoded
|
||||
* value, including commas and quotes, as returned by {@link HttpField#getValue()}.</p>
|
||||
*
|
||||
|
@ -420,6 +429,7 @@ public interface HttpFields extends Iterable<HttpField>, Supplier<HttpFields>
|
|||
* @return the raw value of the first field with the given field name,
|
||||
* or {@code null} if no such field is present
|
||||
* @see HttpField#getValue()
|
||||
* @see #get(HttpHeader)
|
||||
*/
|
||||
default String get(String name)
|
||||
{
|
||||
|
@ -594,12 +604,13 @@ public interface HttpFields extends Iterable<HttpField>, Supplier<HttpFields>
|
|||
* <p>Returns a {@link Set} of the field names.</p>
|
||||
* <p>Case-sensitivity of the field names is preserved.</p>
|
||||
*
|
||||
* @return a {@link Set} of the field names
|
||||
* @return an immutable {@link Set} of the field names. Changes made to the
|
||||
* {@code HttpFields} after this call are not reflected in the set.
|
||||
*/
|
||||
default Set<String> getFieldNamesCollection()
|
||||
{
|
||||
Set<HttpHeader> seenByHeader = EnumSet.noneOf(HttpHeader.class);
|
||||
Set<String> seenByName = null;
|
||||
Set<String> buildByName = null;
|
||||
List<String> list = new ArrayList<>(size());
|
||||
|
||||
for (HttpField f : this)
|
||||
|
@ -607,9 +618,9 @@ public interface HttpFields extends Iterable<HttpField>, Supplier<HttpFields>
|
|||
HttpHeader header = f.getHeader();
|
||||
if (header == null)
|
||||
{
|
||||
if (seenByName == null)
|
||||
seenByName = new TreeSet<>(String::compareToIgnoreCase);
|
||||
if (seenByName.add(f.getName()))
|
||||
if (buildByName == null)
|
||||
buildByName = new TreeSet<>(String::compareToIgnoreCase);
|
||||
if (buildByName.add(f.getName()))
|
||||
list.add(f.getName());
|
||||
}
|
||||
else if (seenByHeader.add(header))
|
||||
|
@ -618,6 +629,8 @@ public interface HttpFields extends Iterable<HttpField>, Supplier<HttpFields>
|
|||
}
|
||||
}
|
||||
|
||||
Set<String> seenByName = buildByName;
|
||||
|
||||
// use the list to retain a rough ordering
|
||||
return new AbstractSet<>()
|
||||
{
|
||||
|
@ -632,6 +645,14 @@ public interface HttpFields extends Iterable<HttpField>, Supplier<HttpFields>
|
|||
{
|
||||
return list.size();
|
||||
}
|
||||
|
||||
@Override
|
||||
public boolean contains(Object o)
|
||||
{
|
||||
if (o instanceof String s)
|
||||
return seenByName != null && seenByName.contains(s) || seenByHeader.contains(HttpHeader.CACHE.get(s));
|
||||
return false;
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
|
|
|
@ -20,12 +20,7 @@ import java.util.Objects;
|
|||
import java.util.stream.Stream;
|
||||
|
||||
/**
|
||||
* HTTP Fields. A collection of HTTP header and or Trailer fields.
|
||||
*
|
||||
* <p>This class is not synchronized as it is expected that modifications will only be performed by a
|
||||
* single thread.
|
||||
*
|
||||
* <p>The cookie handling provided by this class is guided by the Servlet specification and RFC6265.
|
||||
* An immutable implementation of {@link HttpFields}.
|
||||
*/
|
||||
class ImmutableHttpFields implements HttpFields
|
||||
{
|
||||
|
@ -70,10 +65,9 @@ class ImmutableHttpFields implements HttpFields
|
|||
{
|
||||
if (this == o)
|
||||
return true;
|
||||
if (!(o instanceof org.eclipse.jetty.http.ImmutableHttpFields))
|
||||
return false;
|
||||
|
||||
return isEqualTo((HttpFields)o);
|
||||
if (o instanceof HttpFields httpFields)
|
||||
return isEqualTo(httpFields);
|
||||
return false;
|
||||
}
|
||||
|
||||
@Override
|
||||
|
|
|
@ -67,7 +67,7 @@ class MutableHttpFields implements HttpFields.Mutable
|
|||
*/
|
||||
protected MutableHttpFields(HttpFields fields)
|
||||
{
|
||||
if (fields instanceof ImmutableHttpFields immutable)
|
||||
if (fields instanceof org.eclipse.jetty.http.ImmutableHttpFields immutable)
|
||||
{
|
||||
_immutable = true;
|
||||
_fields = immutable._fields;
|
||||
|
@ -180,7 +180,7 @@ class MutableHttpFields implements HttpFields.Mutable
|
|||
public HttpFields asImmutable()
|
||||
{
|
||||
_immutable = true;
|
||||
return new ImmutableHttpFields(_fields, _size);
|
||||
return new org.eclipse.jetty.http.ImmutableHttpFields(_fields, _size);
|
||||
}
|
||||
|
||||
private void copyImmutable()
|
||||
|
|
|
@ -25,6 +25,7 @@ import java.util.ListIterator;
|
|||
import java.util.Locale;
|
||||
import java.util.Map;
|
||||
import java.util.NoSuchElementException;
|
||||
import java.util.Set;
|
||||
import java.util.function.Consumer;
|
||||
import java.util.stream.Collectors;
|
||||
import java.util.stream.Stream;
|
||||
|
@ -352,16 +353,27 @@ public class HttpFieldsTest
|
|||
assertThat(header.get("EXPECT"), is("100"));
|
||||
assertThat(header.get("eXpEcT"), is("100"));
|
||||
assertThat(header.get(HttpHeader.EXPECT), is("100"));
|
||||
assertTrue(header.contains("expect"));
|
||||
assertTrue(header.contains("Expect"));
|
||||
assertTrue(header.contains("EXPECT"));
|
||||
assertTrue(header.contains("eXpEcT"));
|
||||
|
||||
assertThat(header.get("random"), is("value"));
|
||||
assertThat(header.get("Random"), is("value"));
|
||||
assertThat(header.get("RANDOM"), is("value"));
|
||||
assertThat(header.get("rAnDoM"), is("value"));
|
||||
assertThat(header.get("RaNdOm"), is("value"));
|
||||
assertTrue(header.contains("random"));
|
||||
assertTrue(header.contains("Random"));
|
||||
assertTrue(header.contains("RANDOM"));
|
||||
assertTrue(header.contains("rAnDoM"));
|
||||
assertTrue(header.contains("RaNdOm"));
|
||||
|
||||
assertThat(header.get("Accept-Charset"), is("UTF-8"));
|
||||
assertThat(header.get("accept-charset"), is("UTF-8"));
|
||||
assertThat(header.get(HttpHeader.ACCEPT_CHARSET), is("UTF-8"));
|
||||
assertTrue(header.contains("Accept-Charset"));
|
||||
assertTrue(header.contains("accept-charset"));
|
||||
|
||||
assertThat(header.getValuesList("Accept-Charset"), contains("UTF-8", "UTF-16"));
|
||||
assertThat(header.getValuesList("accept-charset"), contains("UTF-8", "UTF-16"));
|
||||
|
@ -371,9 +383,19 @@ public class HttpFieldsTest
|
|||
assertThat(header.get("Foo-Bar"), is("one"));
|
||||
assertThat(header.getValuesList("foo-bar"), contains("one", "two"));
|
||||
assertThat(header.getValuesList("Foo-Bar"), contains("one", "two"));
|
||||
assertTrue(header.contains("foo-bar"));
|
||||
assertTrue(header.contains("Foo-Bar"));
|
||||
|
||||
// We know the order of the set is deterministic
|
||||
assertThat(header.getFieldNamesCollection(), contains("expect", "RaNdOm", "Accept-Charset", "foo-bar"));
|
||||
Set<String> names = header.getFieldNamesCollection();
|
||||
assertThat(names, contains("expect", "RaNdOm", "Accept-Charset", "foo-bar"));
|
||||
assertTrue(names.contains("expect"));
|
||||
assertTrue(names.contains("Expect"));
|
||||
assertTrue(names.contains("random"));
|
||||
assertTrue(names.contains("accept-charset"));
|
||||
assertTrue(names.contains("Accept-Charset"));
|
||||
assertTrue(names.contains("foo-bar"));
|
||||
assertTrue(names.contains("Foo-Bar"));
|
||||
}
|
||||
|
||||
@ParameterizedTest
|
||||
|
|
|
@ -0,0 +1,151 @@
|
|||
//
|
||||
// ========================================================================
|
||||
// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others.
|
||||
//
|
||||
// This program and the accompanying materials are made available under the
|
||||
// terms of the Eclipse Public License v. 2.0 which is available at
|
||||
// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0
|
||||
// which is available at https://www.apache.org/licenses/LICENSE-2.0.
|
||||
//
|
||||
// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0
|
||||
// ========================================================================
|
||||
//
|
||||
|
||||
package org.eclipse.jetty.server.jmh;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.EnumMap;
|
||||
import java.util.HashMap;
|
||||
import java.util.List;
|
||||
import java.util.Map;
|
||||
import java.util.concurrent.TimeUnit;
|
||||
|
||||
import org.eclipse.jetty.http.HttpField;
|
||||
import org.eclipse.jetty.http.HttpHeader;
|
||||
import org.openjdk.jmh.annotations.Benchmark;
|
||||
import org.openjdk.jmh.annotations.BenchmarkMode;
|
||||
import org.openjdk.jmh.annotations.Fork;
|
||||
import org.openjdk.jmh.annotations.Measurement;
|
||||
import org.openjdk.jmh.annotations.Mode;
|
||||
import org.openjdk.jmh.annotations.OperationsPerInvocation;
|
||||
import org.openjdk.jmh.annotations.Scope;
|
||||
import org.openjdk.jmh.annotations.State;
|
||||
import org.openjdk.jmh.annotations.Warmup;
|
||||
import org.openjdk.jmh.runner.Runner;
|
||||
import org.openjdk.jmh.runner.RunnerException;
|
||||
import org.openjdk.jmh.runner.options.Options;
|
||||
import org.openjdk.jmh.runner.options.OptionsBuilder;
|
||||
|
||||
@State(Scope.Thread)
|
||||
@BenchmarkMode(Mode.Throughput)
|
||||
@Fork(1)
|
||||
@Warmup(iterations = 6, time = 2000, timeUnit = TimeUnit.MILLISECONDS)
|
||||
@Measurement(iterations = 3, time = 2000, timeUnit = TimeUnit.MILLISECONDS)
|
||||
public class HashMapVsEnumMapBenchmark
|
||||
{
|
||||
private static final HttpHeader[] HEADERS = HttpHeader.values();
|
||||
private static final HttpHeader[] HEADER_NAMES =
|
||||
{
|
||||
// These will be hits
|
||||
HttpHeader.HOST,
|
||||
HttpHeader.CONTENT_TYPE,
|
||||
HttpHeader.CONTENT_LENGTH,
|
||||
HttpHeader.ACCEPT,
|
||||
|
||||
// These will be misses
|
||||
HttpHeader.TRANSFER_ENCODING,
|
||||
HttpHeader.AUTHORIZATION
|
||||
};
|
||||
|
||||
private List<HttpField> newHeaders()
|
||||
{
|
||||
List<HttpField> list = new ArrayList<>();
|
||||
list.add(new HttpField(HttpHeader.HOST, "Localhost"));
|
||||
list.add(new HttpField(HttpHeader.CONTENT_TYPE, "application/json"));
|
||||
list.add(new HttpField(HttpHeader.CONTENT_LENGTH, "123"));
|
||||
list.add(new HttpField(HttpHeader.USER_AGENT, "JMH Benchmark"));
|
||||
list.add(new HttpField(HttpHeader.ACCEPT, "application/json"));
|
||||
return list;
|
||||
}
|
||||
|
||||
@Benchmark
|
||||
@OperationsPerInvocation(5)
|
||||
public long testListLookup()
|
||||
{
|
||||
// Build the HashMap
|
||||
List<HttpField> list = newHeaders();
|
||||
|
||||
// Perform lookups
|
||||
long result = 0;
|
||||
for (HttpHeader header : HEADER_NAMES)
|
||||
{
|
||||
for (HttpField field : list)
|
||||
{
|
||||
if (field.getHeader() == header)
|
||||
{
|
||||
result ^= field.getValue().hashCode();
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
@Benchmark
|
||||
@OperationsPerInvocation(5)
|
||||
public long testHashMapBuildAndLookup()
|
||||
{
|
||||
// Build the HashMap
|
||||
List<HttpField> list = newHeaders();
|
||||
Map<String, HttpField> hashMap = new HashMap<>();
|
||||
for (HttpField field : list)
|
||||
{
|
||||
hashMap.put(field.getName(), field);
|
||||
}
|
||||
|
||||
// Perform lookups
|
||||
long result = 0;
|
||||
for (HttpHeader header : HEADER_NAMES)
|
||||
{
|
||||
HttpField field = hashMap.get(header.asString());
|
||||
if (field != null)
|
||||
result ^= field.getValue().hashCode();
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
@Benchmark
|
||||
@OperationsPerInvocation(5)
|
||||
public long testEnumMapBuildAndLookup()
|
||||
{
|
||||
// Build the EnumMap
|
||||
Map<HttpHeader, HttpField> enumMap = new EnumMap<>(HttpHeader.class);
|
||||
|
||||
List<HttpField> list = newHeaders();
|
||||
for (HttpField field : list)
|
||||
{
|
||||
enumMap.put(field.getHeader(), field);
|
||||
}
|
||||
|
||||
// Perform lookups
|
||||
long result = 0;
|
||||
for (HttpHeader header : HEADERS)
|
||||
{
|
||||
HttpField field = enumMap.get(header);
|
||||
if (field != null)
|
||||
result ^= field.getValue().hashCode();
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
public static void main(String[] args) throws RunnerException
|
||||
{
|
||||
Options opt = new OptionsBuilder()
|
||||
.include(HashMapVsEnumMapBenchmark.class.getSimpleName())
|
||||
// .addProfiler(GCProfiler.class)
|
||||
.forks(1)
|
||||
.build();
|
||||
|
||||
new Runner(opt).run();
|
||||
}
|
||||
}
|
Loading…
Reference in New Issue