Skip to content

Commit e18248f

Browse files
ok2cok2c
committed
Optimized H2 priority formatter to produce less intermediate garbage; removed unused H2 priority related code
1 parent 13b473c commit e18248f

6 files changed

Lines changed: 90 additions & 379 deletions

File tree

‎httpcore5-h2/src/main/java/org/apache/hc/core5/http2/impl/nio/AbstractH2StreamMultiplexer.java‎

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
import org.apache.hc.core5.http.Header;
5353
import org.apache.hc.core5.http.HttpConnection;
5454
import org.apache.hc.core5.http.HttpException;
55+
import org.apache.hc.core5.http.HttpHeaders;
5556
import org.apache.hc.core5.http.HttpStreamResetException;
5657
import org.apache.hc.core5.http.HttpVersion;
5758
import org.apache.hc.core5.http.ProtocolException;
@@ -71,7 +72,6 @@
7172
import org.apache.hc.core5.http.nio.command.StaleCheckCommand;
7273
import org.apache.hc.core5.http.protocol.HttpContext;
7374
import org.apache.hc.core5.http.protocol.HttpProcessor;
74-
import org.apache.hc.core5.http.HttpHeaders;
7575
import org.apache.hc.core5.http2.H2ConnectionException;
7676
import org.apache.hc.core5.http2.H2Error;
7777
import org.apache.hc.core5.http2.H2StreamResetException;
@@ -91,7 +91,6 @@
9191
import org.apache.hc.core5.http2.nio.command.PushResponseCommand;
9292
import org.apache.hc.core5.http2.priority.PriorityParamsParser;
9393
import org.apache.hc.core5.http2.priority.PriorityValue;
94-
import org.apache.hc.core5.http2.priority.PriorityFormatter;
9594
import org.apache.hc.core5.io.CloseMode;
9695
import org.apache.hc.core5.reactor.Command;
9796
import org.apache.hc.core5.reactor.ProtocolIOSession;
@@ -1369,25 +1368,6 @@ H2Stream createStream(final H2StreamChannel channel, final H2StreamHandler strea
13691368
return streams.createActive(channel, streamHandler);
13701369
}
13711370

1372-
public final void sendPriorityUpdate(final int prioritizedStreamId, final PriorityValue value) throws IOException {
1373-
if (value == null) {
1374-
return;
1375-
}
1376-
final String field = PriorityFormatter.format(value);
1377-
if (field == null) {
1378-
return;
1379-
}
1380-
final byte[] ascii = field.getBytes(StandardCharsets.US_ASCII);
1381-
final ByteArrayBuffer buf = new ByteArrayBuffer(4 + ascii.length);
1382-
buf.append((byte) (prioritizedStreamId >> 24));
1383-
buf.append((byte) (prioritizedStreamId >> 16));
1384-
buf.append((byte) (prioritizedStreamId >> 8));
1385-
buf.append((byte) prioritizedStreamId);
1386-
buf.append(ascii, 0, ascii.length);
1387-
final RawFrame frame = frameFactory.createPriorityUpdate(ByteBuffer.wrap(buf.array(), 0, buf.length()));
1388-
commitFrame(frame);
1389-
}
1390-
13911371
private void recordPriorityFromHeaders(final int streamId, final List<? extends Header> headers) {
13921372
if (headers == null || headers.isEmpty()) {
13931373
return;

‎httpcore5-h2/src/main/java/org/apache/hc/core5/http2/priority/PriorityFormatter.java‎

Lines changed: 36 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,12 @@
2727
package org.apache.hc.core5.http2.priority;
2828

2929

30-
import java.util.ArrayList;
31-
import java.util.List;
32-
3330
import org.apache.hc.core5.annotation.Internal;
31+
import org.apache.hc.core5.http.Header;
32+
import org.apache.hc.core5.http.HttpHeaders;
33+
import org.apache.hc.core5.http.message.BasicHeader;
34+
import org.apache.hc.core5.http.message.BufferedHeader;
35+
import org.apache.hc.core5.util.CharArrayBuffer;
3436

3537
/**
3638
* Formats PriorityValue as RFC 9218 Structured Fields Dictionary.
@@ -43,21 +45,44 @@ public final class PriorityFormatter {
4345
private PriorityFormatter() {
4446
}
4547

46-
public static String format(final PriorityValue value) {
48+
public static void format(final CharArrayBuffer dst, final PriorityValue value) {
4749
if (value == null) {
48-
return null;
50+
return;
4951
}
50-
final List<String> parts = new ArrayList<>(2);
52+
boolean urgencyPresent = false;
5153
if (value.getUrgency() != PriorityValue.DEFAULT_URGENCY) {
52-
parts.add("u=" + value.getUrgency());
54+
dst.append("u=");
55+
dst.append(value.getUrgency());
56+
urgencyPresent = true;
5357
}
5458
if (value.isIncremental()) {
5559
// In SF Dictionary, boolean true can be represented by key without value (per RFC 8941).
56-
parts.add("i");
60+
if (urgencyPresent) {
61+
dst.append(", ");
62+
}
63+
dst.append("i");
64+
}
65+
66+
}
67+
68+
public static String format(final PriorityValue value) {
69+
if (value == null) {
70+
return null;
5771
}
58-
if (parts.isEmpty()) {
59-
return null; // omit header when all defaults
72+
final CharArrayBuffer buf = new CharArrayBuffer(16);
73+
format(buf, value);
74+
return buf.toString();
75+
}
76+
77+
public static Header formatHeader(final PriorityValue value) {
78+
if (value == null) {
79+
return new BasicHeader(HttpHeaders.PRIORITY, null);
6080
}
61-
return String.join(", ", parts);
81+
final CharArrayBuffer buf = new CharArrayBuffer(16);
82+
buf.append(HttpHeaders.PRIORITY);
83+
buf.append(": ");
84+
format(buf, value);
85+
return BufferedHeader.create(buf);
6286
}
87+
6388
}

‎httpcore5-h2/src/main/java/org/apache/hc/core5/http2/protocol/H2RequestPriority.java‎

Lines changed: 0 additions & 121 deletions
This file was deleted.

httpcore5-h2/src/test/java/org/apache/hc/core5/http2/examples/ClassicH2PriorityExample.java renamed to httpcore5-h2/src/test/java/org/apache/hc/core5/http2/examples/H2RequestExecutionWithPriorityExample.java

Lines changed: 53 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -26,34 +26,31 @@
2626
*/
2727
package org.apache.hc.core5.http2.examples;
2828

29-
import java.io.BufferedReader;
30-
import java.io.InputStreamReader;
31-
import java.nio.charset.Charset;
32-
import java.nio.charset.StandardCharsets;
3329
import java.util.List;
30+
import java.util.concurrent.CountDownLatch;
3431
import java.util.concurrent.Future;
3532

3633
import org.apache.hc.core5.annotation.Experimental;
37-
import org.apache.hc.core5.http.ClassicHttpRequest;
38-
import org.apache.hc.core5.http.ClassicHttpResponse;
39-
import org.apache.hc.core5.http.ContentType;
34+
import org.apache.hc.core5.concurrent.FutureCallback;
4035
import org.apache.hc.core5.http.Header;
4136
import org.apache.hc.core5.http.HttpConnection;
42-
import org.apache.hc.core5.http.HttpEntity;
4337
import org.apache.hc.core5.http.HttpHost;
38+
import org.apache.hc.core5.http.HttpResponse;
39+
import org.apache.hc.core5.http.Message;
4440
import org.apache.hc.core5.http.impl.bootstrap.HttpAsyncRequester;
45-
import org.apache.hc.core5.http.io.support.ClassicRequestBuilder;
41+
import org.apache.hc.core5.http.message.BasicHttpRequest;
4642
import org.apache.hc.core5.http.nio.AsyncClientEndpoint;
47-
import org.apache.hc.core5.http.nio.support.classic.ClassicToAsyncRequestProducer;
48-
import org.apache.hc.core5.http.nio.support.classic.ClassicToAsyncResponseConsumer;
49-
import org.apache.hc.core5.http.protocol.HttpCoreContext;
43+
import org.apache.hc.core5.http.nio.entity.StringAsyncEntityConsumer;
44+
import org.apache.hc.core5.http.nio.support.BasicRequestProducer;
45+
import org.apache.hc.core5.http.nio.support.BasicResponseConsumer;
46+
import org.apache.hc.core5.http.support.BasicRequestBuilder;
5047
import org.apache.hc.core5.http2.HttpVersionPolicy;
5148
import org.apache.hc.core5.http2.config.H2Config;
5249
import org.apache.hc.core5.http2.frame.RawFrame;
5350
import org.apache.hc.core5.http2.impl.H2Processors;
5451
import org.apache.hc.core5.http2.impl.nio.H2StreamListener;
52+
import org.apache.hc.core5.http2.priority.PriorityFormatter;
5553
import org.apache.hc.core5.http2.priority.PriorityValue;
56-
import org.apache.hc.core5.http2.protocol.H2RequestPriority;
5754
import org.apache.hc.core5.io.CloseMode;
5855
import org.apache.hc.core5.util.Timeout;
5956

@@ -63,7 +60,7 @@
6360
* Requires H2Processors to include H2RequestPriority (client chain) and an HTTP/2 connection.
6461
*/
6562
@Experimental
66-
public class ClassicH2PriorityExample {
63+
public class H2RequestExecutionWithPriorityExample {
6764

6865
public static void main(final String[] args) throws Exception {
6966

@@ -120,50 +117,62 @@ public void onOutputFlowControl(final HttpConnection c, final int id, final int
120117
final Future<AsyncClientEndpoint> future = requester.connect(target, Timeout.ofSeconds(30));
121118
final AsyncClientEndpoint clientEndpoint = future.get();
122119

120+
final CountDownLatch latch = new CountDownLatch(2);
121+
123122
// ---- Request 1: Explicit non-default priority -> header MUST be emitted
124-
executeWithPriority(clientEndpoint, target, "/httpbin/headers", PriorityValue.of(0, true));
123+
executeWithPriority(clientEndpoint, target, "/httpbin/headers", PriorityValue.of(0, true), latch);
125124

126125
// ---- Request 2: RFC defaults -> header MUST be omitted by the interceptor
127-
executeWithPriority(clientEndpoint, target, "/httpbin/user-agent", PriorityValue.defaults());
126+
executeWithPriority(clientEndpoint, target, "/httpbin/user-agent", PriorityValue.defaults(), latch);
128127

129128
System.out.println("Shutting down I/O reactor");
130129
requester.initiateShutdown();
131130
}
132131

133132
private static void executeWithPriority(
134-
final AsyncClientEndpoint endpoint,
133+
final AsyncClientEndpoint clientEndpoint,
135134
final HttpHost target,
136-
final String path,
137-
final PriorityValue priorityValue) throws Exception {
135+
final String requestUri,
136+
final PriorityValue priorityValue,
137+
final CountDownLatch latch) throws Exception {
138138

139-
final ClassicHttpRequest request = ClassicRequestBuilder.get()
139+
final BasicHttpRequest request = BasicRequestBuilder.get()
140140
.setHttpHost(target)
141-
.setPath(path)
141+
.setPath(requestUri)
142142
.build();
143+
if (!PriorityValue.defaults().equals(priorityValue)) {
144+
request.addHeader(PriorityFormatter.formatHeader(priorityValue));
145+
}
146+
147+
clientEndpoint.execute(
148+
new BasicRequestProducer(request, null),
149+
new BasicResponseConsumer<>(new StringAsyncEntityConsumer()),
150+
new FutureCallback<Message<HttpResponse, String>>() {
143151

144-
// Place the PriorityValue into the context so H2RequestPriority can format the header
145-
final HttpCoreContext ctx = HttpCoreContext.create();
146-
ctx.setAttribute(H2RequestPriority.ATTR_HTTP2_PRIORITY_VALUE, priorityValue);
147-
148-
final ClassicToAsyncRequestProducer requestProducer = new ClassicToAsyncRequestProducer(request, Timeout.ofMinutes(1));
149-
final ClassicToAsyncResponseConsumer responseConsumer = new ClassicToAsyncResponseConsumer(Timeout.ofMinutes(1));
150-
151-
endpoint.execute(requestProducer, responseConsumer, ctx, null);
152-
153-
requestProducer.blockWaiting().execute();
154-
try (ClassicHttpResponse response = responseConsumer.blockWaiting()) {
155-
System.out.println(path + " -> " + response.getCode());
156-
final HttpEntity entity = response.getEntity();
157-
if (entity != null) {
158-
final ContentType ct = ContentType.parse(entity.getContentType());
159-
final Charset cs = ContentType.getCharset(ct, StandardCharsets.UTF_8);
160-
try (BufferedReader reader = new BufferedReader(new InputStreamReader(entity.getContent(), cs))) {
161-
String line;
162-
while ((line = reader.readLine()) != null) {
163-
System.out.println(line);
152+
@Override
153+
public void completed(final Message<HttpResponse, String> message) {
154+
clientEndpoint.releaseAndReuse();
155+
final HttpResponse response = message.getHead();
156+
final String body = message.getBody();
157+
System.out.println(requestUri + "->" + response.getCode());
158+
System.out.println(body);
159+
latch.countDown();
164160
}
165-
}
166-
}
167-
}
161+
162+
@Override
163+
public void failed(final Exception ex) {
164+
clientEndpoint.releaseAndDiscard();
165+
System.out.println(requestUri + "->" + ex);
166+
latch.countDown();
167+
}
168+
169+
@Override
170+
public void cancelled() {
171+
clientEndpoint.releaseAndDiscard();
172+
System.out.println(requestUri + " cancelled");
173+
latch.countDown();
174+
}
175+
176+
});
168177
}
169178
}

0 commit comments

Comments
 (0)