Commit Graph

6592 Commits

Author SHA1 Message Date
Jakob Buchgraber
42bcaef4a4 Avoid object allocations for HTTP2 child streams.
Motivation:

We are allocating a hash map for every HTTP2 Stream to store it's children.
Most streams are leafs in the priority tree and don't have children.

Modification:

 - Only allocate children when we actually use them.
 - Make EmptyIntObjectMap not throw a UnsupportedOperationException on remove, but return null instead (as is stated in it's javadoc).

Result:

Fewer unnecessary allocations.
2015-04-03 11:49:01 -07:00
Jakob Buchgraber
5de91c0c7a Replace LinkedHashSet by ArrayList to avoid iterators.
Motivation:

In a simple load test that creates and closes several 10k streams per second
I have seen Iterator objects using roughly 1.6% of the total committed heap.

Modifications:

Use an ArrayList instead of a LinkedHashSet to store the connection listeners.
That way we can iterate over the list without creating an iterator every time.

Result:

Zero Iterator allocations due to notifying connection listeners.
2015-04-03 20:09:10 +02:00
nmittler
96cd447ce9 Allow non-standard HTTP/2 settings
Motivation:

The Http2Settings class currently disallows setting non-standard settings, which violates the spec.

Modifications:

Updated Http2Settings to permit arbitrary settings. Also adjusting the default initial capacity to allow setting all of the standard settings without reallocation.

Result:

Fixes #3560
2015-04-02 11:10:20 -07:00
Trustin Lee
cdd2e1c8c1 Fix unbounded expansion of cumulative buffer in SslHandler
Related: #3567

Motivation:

SslHandler.channelReadComplete() forgets to call
super.channelReadComplete(), which discards read bytes from the
cumulative buffer.  As a result, the cumulative buffer can expand its
capacity unboundedly.

Modifications:

Call super.channelReadComplete() instead of calling
ctx.fireChannelReadComplete()

Result:

Fixes #3567
2015-04-02 14:57:40 +09:00
Scott Mitchell
2b3e0e675a HTTP/2 Closed Streams Conditional Priority Tree Removal
Motivation:
The HTTP/2 specification allows for closed (and streams in any state) to exist in the priority tree. The current code removes streams from the priority tree as soon as they are closed (subject to the removal policy). This may lead to undesired distribution of resources from the peer's perspective.

Modifications:
- We should only remove streams from the priority tree when they have no descendant streams in a viable state.
- We should track when tree edges change or nodes are removed if inviable nodes can then be removed.

Result:
Priority tree doesn't remove closed streams until descendant are all closed, or there are no descendants.
2015-03-31 16:25:00 -07:00
nmittler
2faa473b82 Removing unnecessary use of Math.ceil in HTTP/2 priority algorithm.
Motivation:

We're currently using Math.ceil which isn't necessary. We should exchange for a lighter weight operation.

Modifications:

Changing the logic to just ensure that we allocate at least one byte to the child rather than always performing a ceil.

Result:

Slight performance improvement in the priority algorithm.
2015-03-31 11:02:49 -07:00
nmittler
6ebcd089df Include error code and message in GOAWAY events.
Motivation:

The Connection.Listener GOAWAY event handler currently provides no additional information, requiring applications to hack in other ways to get at the error code and debug message.

Modifications:

Modified the Connection.Listener interface to pass on the error code and message that triggered the GOAWAY.

Result:

Application can now use Connection.Listener for all GOAWAY processing.
2015-03-31 09:17:49 -07:00
Trustin Lee
d7969205c3 Fix intermittent test failure in LoggingHandlerTest
Motivation:

LoggingHandlerTest sometimes failure due to unexpected log messages
logged due to the automatic reclaimation of thread-local objects.

  Expectation failure on verify:
    Appender.doAppend([DEBUG] Freed 3 thread-local buffer(s) from thread: nioEventLoopGroup-23-0): expected: 1, actual: 0
    Appender.doAppend([DEBUG] Freed 9 thread-local buffer(s) from thread: nioEventLoopGroup-23-1): expected: 1, actual: 0
    Appender.doAppend([DEBUG] Freed 2 thread-local buffer(s) from thread: nioEventLoopGroup-23-2): expected: 1, actual: 0
    Appender.doAppend([DEBUG] Freed 4 thread-local buffer(s) from thread: nioEventLoopGroup-26-0): expected: 1, actual: 0
    Appender.doAppend(matchesLog(expected: ".+CLOSE$", got: "[id: 0xembedded, embedded => embedded] CLOSE")): expected: 1, actual: 0

Modifications:

Add the mock appender to the related logger only

Result:

No more intermittent test failures
2015-03-31 15:13:30 +09:00
Trustin Lee
f1f7f92324 Don't trigger IOException at ChunkedStream.isEndOfInput()
Related: #3368

Motivation:

ChunkedWriteHandler checks if the return value of
ChunkedInput.isEndOfInput() after calling ChunkedInput.close().

This makes ChunkedStream.isEndOfInput() trigger an IOException, which is
originally triggered by PushBackInputStream.read().

By contract, ChunkedInput.isEndOfInput() should not raise an IOException
even when the underlying stream is closed.

Modifications:

Add a boolean flag that keeps track of whether the underlying stream has
been closed or not, so that ChunkedStream.isEndOfInput() does not
propagate the IOException from PushBackInputStream.

Result:

Fixes #3368
2015-03-31 11:38:41 +09:00
nmittler
6bf58255bc Cleaning up the initialization of Http2ConnectionHandler
Motivation:

It currently takes a builder for the encoder and decoder, which makes it difficult to decorate them.

Modifications:

Removed the builders from the interfaces entirely. Left the builder for the decoder impl but removed it from the encoder since it's constructor only takes 2 parameters. Also added decorator base classes for the encoder and decoder and made the CompressorHttp2ConnectionEncoder extend the decorator.

Result:

Fixes #3530
2015-03-30 11:22:25 -07:00
nmittler
1e7eabc58c Removing unnecessary sort in remote flow controller.
Motivation:

The DefaultHttp2RemoteFlowController's priority algorithm doesn't really need to sort the children by weight since it already fairly distributes data based on weight.

Modifications:

Removing the sorting in the priority algorithm and updating one test to allow a small bit of variability in the results.

Result:

Slight improvement on the performance of the priority algorithm.
2015-03-30 09:54:33 -07:00
Norman Maurer
b6bf683a3e Add supported for X509ExtendedTrustManager when using OpenSslEngine
Motivation:

For some use cases X509ExtendedTrustManager is needed as it allows to also access the SslEngine during validation.

Modifications:

Add support for X509ExtendedTrustManager on java >= 7

Result:

It's now possible to use X509ExtendedTrustManager with OpenSslEngine
2015-03-30 09:05:28 +02:00
Scott Mitchell
bd2cbe6a35 Http/2 Priority on CLOSED stream
Motivation:
The encoder/decoder currently do not handle streams which have previously existed but no longer exist because they were closed. The specification requires supporting this.

Modifications:
- encoder/decoder should tolerate the frame or the dependent frame not existing in the streams map due to the fact that it may have previously existed.

Result:
encoder/decoder are more compliant with the specification.
2015-03-28 19:12:26 -07:00
Scott Mitchell
4408180d29 HTTP/2 Decoder reduce preface conditional checks
Motivation:
The DefaultHttp2ConnectionDecoder class is calling verifyPrefaceReceived() for almost every frame event at all times.
The Http2ConnectionHandler class is calling readClientPrefaceString() on every decode event.

Modifications:
- DefaultHttp2ConnectionDecoder should not have to continuously call verifyPrefaceReceived() because it transitions boolean state 1 time for each connection.
- Http2ConnectionHandler should not have to continuously call readClientPrefaceString() because it transitions boolean state 1 time for each connection.

Result:
- Less conditional checks for the mainstream usage of the connection.
2015-03-28 19:06:10 -07:00
nmittler
e1c24fd4e5 Decoupling allocation from writing in HTTP/2 outbound flow control
Motivation:

The current DefaultHttp2RemoteFlowController's writePendingBytes currently operates in 2 passes. The first allocates bytes and optionally writes some frames. The second pass just loops across all active streams and writes all remaining bytes.

If streams can be removed/added as a side effect of writing (EOS or error) then we need to take more care when the write actually occurs. Moving all of the writes to the second loop (across active streams) is simpler since we can just make a copy of the list and not worry about any restructuring of the priority tree that may result.

Modifications:

Modified DefaultHttp2RemoteFlowController.writePendingBytes to only allocate bytes on the first pass and then write any allocated bytes on the second pass.

Result:

Side effects resulting from writing should no longer impact the flow control algorithm.
2015-03-28 14:18:49 -07:00
scottmitch
a6c729bdf8 Http2DefaultFrameWriter microbenchmark
Motivation:
A microbenchmark will be useful to get a baseline for performance.

Modifications:
- Introduce a new microbenchmark which tests the Http2DefaultFrameWriter.
- Allow benchmarks to run without thread context switching between JMH and Netty.

Result:
Microbenchmark exists to test performance.
2015-03-27 13:09:36 -07:00
Scott Mitchell
c60d1f3206 Comment punctuation cleanup
Motivation:
Commit d857b16d76 introduced some comments that had no punctuation.

Modifications:
Add punctuation.

Result:
Comments have punctuation.
2015-03-26 13:03:03 -07:00
Scott Mitchell
bd66dca418 Http/2 RST_STREAM frame robustness
Motivation:
The Http2ConnectionHandler writeRstStream method allows RST_STREAM frames to be sent when we do not know about the stream and after a RST_STREAM frame has already been sent.  This may lead to sending frames when we should not according to the HTTP/2 spec. There is also the potential to notify the closeListener multiple times if the closeStream method is called multiple times.

Modifications:
- Prevent RST_STREAM from being sent if we don't know about the stream, or if we already sent the RST_STREAM.
- Prevent the closeListener from being notified multiple times.

Result:
More robust writeRstStream logic in boundary conditions.
2015-03-26 11:47:47 -07:00
Scott Mitchell
4ff551baa2 Http2 draft 17
Motivation:
There was a new draft for HTTP/2.  We should support the new draft.

Modifications:
- Review the HTTP/2 draft 17 specification, and update code to reflect changes.

Result:
Support for HTTP/2 draft 17.
2015-03-25 09:02:58 -07:00
Scott Mitchell
c81937dff3 Jetty ALNPN and NPN updates plus backport
Motivation:
There are new versions of the ALPN and NPN dependencies.  There was also some backport misses in the pom file related to ALPN/NPN.

Modifications:
- Add new versions for ALPN/NPN dependencies.
- Backport missed pieces from pom.xml.

Result:
Updated version of ALPN/NPN versions.
2015-03-25 08:57:51 -07:00
Pierre DAL-PRA
d7581e5726 Small typos fixes in Channel's Javadoc 2015-03-21 16:09:55 +01:00
Jakob Buchgraber
c884847af9 Have FlowState.cancel take a Throwable and code cleanup.
Motivation:

- In FlowState.write(...) we are currently swalloing an exception.
- In my previous commit I introduced a compiler warning by not making
  a local variabe final.

Modifications:

- Have FlowState.cancel() take a Throwable.
- Make the variable final.

Result:

No more swallowed exceptions and warnings.
2015-03-19 22:01:49 -07:00
Jakob Buchgraber
af83b56e59 HTTP2: Close encoder and decoder on channelInactive and initialize clientPrefaceString on handlerAdded.
Motivation:

- The encoder and decoder should be closed right after the handler releases its resources.
- The clientPrefaceString is allocated in the constructor but releases in handlerRemoved.
  If the handler is never added to the pipeline, the clientPrefaceString will never be
  released.

Modifications:

- Call encoder.close() and decoder.close() on channelInactive.
- Release the clientPrefaceString on handlerRemoved.

Result:

- The encoder and decoder get closed right after the handler's resources are freed.
- It's easier to verify that the clientPrefaceString will also get released.
2015-03-19 13:09:05 -07:00
JongYoonLim
8cbc46a301 Returns after encoding each message not do check following instance types
Motivation:
Current AbstractMemcacheObjectEncoder does unnecessary message type checking if the message is MemcacheMessage type.

Modifications:
Returns after encoding MemcacheMessage message.

Result:
Small performance improvement for this encoder.
2015-03-19 20:43:10 +01:00
JongYoonLim
5dd3e883b1 fix typo 2015-03-19 20:40:13 +01:00
Wouter
59a042aae4 Fix typo in javadoc 2015-03-18 17:14:54 +01:00
Trustin Lee
42cd55fca5 Safely encode Strings to ASCII
(Ported @luciferous's changes against 3.10)

Motivation:

The current implementation of the encoder writes each character of the
String as a single byte to the buffer, however not all characters are
mappable to a single byte.

Modifications:

If a character is outside the ASCII range, it's converted to '?'.

Result:

A safer encoder for String to ASCII, which substitutes unmappable
characters with'?'.
2015-03-18 15:53:52 +09:00
nmittler
c14e6597ae Using public LogLevel for HTTP/2 frame logging.
Motivation:

The Http2FrameLogger is currently using the internal logging classes. We should change this so that it's using the public classes and then converts internally.

Modifications:

Modified Http2FrameLogger and the examples to use the public LogLevel class.

Result:

Fixes #2512
2015-03-17 14:58:44 -07:00
Idel Pivnitskiy
9ec4c39186 Update jUnit version to 4.12
Motivation:

Too many new features in the new release of jUnit.
https://github.com/junit-team/junit/blob/master/doc/ReleaseNotes4.12.md

Modifications:

- Changed version of jUnit from 4.11 to 4.12 in the parent pom.

Result:

Allows using new testing features.
2015-03-17 15:50:50 +01:00
Leonardo Freitas Gomes
4f13dee454 Ensure server preference order in ALPN
Motivation:
With the current implementation the client protocol preference list
takes precedence over the one of the server, since the select method
will return the first item, in the client list, that matches any of the
protocols supported by the server. This violates the recommendation of
http://tools.ietf.org/html/rfc7301#section-3.2.

It will also fail with the current implementation of Chrome, which
sends back Extension application_layer_protocol_negotiation, protocols:
[http/1.1, spdy/3.1, h2-14]

Modifications:
Changed the protocol negotiator to prefer server’s list. Added a test
case that demonstrates the issue and that is fixed with the
modifications of this commit.

Result:
Server’s preference list is used.
2015-03-17 07:30:17 +01:00
nmittler
b7f57223c1 Cleaning up HTTP/2 method names for max_concurrent_streams
Motivation:

The current documentation for Endpoint methods referring to concurrent streams and the SETTINGS_MAX_CONCURRENT_STREAMS setting are a bit confusing.

Modifications:

Renamed a few of the methods and added more clear documentation.

Result:

Fixes #3451
2015-03-16 10:17:05 -07:00
Jakob Buchgraber
6e894c411b Remove dead code from DefaultHttp2ConnectionEncoder.
Motivation:

There are two writeRstStream methods in the DefaultHttp2ConnectionEncoder.
One of the two is neither used nor part of the Http2FrameWriter interface.

Modifications:

Delete the method.

Result:

Fewer lines of dead code.
2015-03-14 09:02:14 -07:00
Jakob Buchgraber
d91cae2fc7 Remove Frame class from DefaultHttp2RemoteFlowController. Fixes #3488
Motivation:

For every write of a flow controlled frame (HEADERS, DATA) we are allocating
a Frame object that is not necessary anymore as it does not maintain any
state, besides the payload.

Modifications:

Remove the Frame class and directly add the payload to the pending write queue.

Result:

One few object allocation per write of a flow controlled frame.
2015-03-13 15:53:28 -07:00
Robert.Panzer
08b1438e7b Add support for byte order to LengthFieldPrepender
Motivation:

While the LengthFieldBasedFrameDecoder supports a byte order the LengthFieldPrepender does not.
That means that I can simply add a LengthFieldBasedFrameDecoder with ByteOrder.LITTLE_ENDIAN to my pipeline
but have to write my own Encoder to write length fields in little endian byte order.

Modifications:

Added a constructor that takes a byte order and all other parameters.
All other constructors delegate to this one with ByteOrder.BIG_ENDIAN.
LengthFieldPrepender.encode() uses this byte order to write the length field.

Result:

LengthFieldPrepender will write the length field in the defined byte order.
2015-03-13 18:37:58 +01:00
Norman Maurer
63859a550f First load RuntimeException to prevent segfault on error
Motivation:

When an error happens during loading the native library it may try to generate a new RuntimeException before the RuntimeException is loaded.

Modifications:

- Load RuntimeException as first

Result:

No more segfaults possible
2015-03-13 18:28:49 +01:00
nmittler
d3071bb1d8 Optimizations for Http2FrameLogger
Motivation:

The logger was always performing a hex dump of the ByteBufs regarless whether or not the log would take place.

Modifications:

Fixed the logger to avoid serializing the ByteBufs and calling the varargs method if logging is not enabled.

Result:

The loggers should run MUCH faster when disabled.
2015-03-12 14:18:34 -07:00
Jakob Buchgraber
93bcc6bbde Document nEventLoops=0 in (Nio|Default|Epoll)EventLoopGroups.
Motivation:

When setting nEventLoops to zero in the MultithreadedEventLoopGroup constructor
the EventLoopGroup chooses the number of EventLoops and Threads to use for you.
We want to make use of this behaviour internally and thus we would like to document
it as a part of the official API, so that we can rely on it.

Modifications:

Document the behaviour when setting nEventLoops to zero.
Fix several spelling / sloppiness mistakes in the documentation.

Result:

nEventLoops=0 is now documented as a part of the official API.
The quality of the documentation is improved.
2015-03-11 11:21:58 +09:00
Norman Maurer
88954c022c Respect -Djava.net.preferIPv4Stack when using epoll transport
Motivation:

On a system where ipv4 and ipv6 are supported a user may want to use -Djava.net.preferIPv4Stack=true to restrict it to use ipv4 only.
This is currently ignored with the epoll transport.

Modifications:

Respect java.net.preferIPv4Stack system property.

Result:

-Djava.net.preferIPv4Stack=true will have the effect the user is looking for.
2015-03-11 02:50:37 +01:00
Jakob Buchgraber
44ee2cac43 Fix premature cancelation of pending frames in HTTP2 Flow Control.
Motivation:

If HEADERS or DATA frames are pending due to a too small flow control
window, a frame with the END_STREAM flag set will wrongfully cancel
all pending frames (including itself).

Also see grpc/grpc-java#145

Modifications:

The transition of the stream state to CLOSE / HALF_CLOSE due to a
set END_STREAM flag is delayed until the frame with the flag is
actually written to the Channel.

Result:

Flow control works correctly. Frames with END_STREAM flag will no
longer cancel their preceding frames.
2015-03-10 12:34:08 -07:00
Norman Maurer
4d350d31a6 Fix possible AttributeMap corruption on double removal
Motivation:

When remove0() is called multiple times for an DefaultAttribute it can cause corruption of the internal linked-list structure.

Modifications:

- Ensure remove0() can not cause corruption by null out prev and next references.

Result:

No more corruption possible
2015-03-10 05:09:06 +01:00
Trustin Lee
4aa10db9c5 Use InetSocketAddress.getHostName() instead of getHostString()
Related: #3478

Motivation:

DefaultNameResolver uses InetSocketAddress.getHostString() instead of
getHostName(). Because Netty uses the DefaultNameResolver by default and
getHostString() is available only since Java 7, a user cannot use Netty
on Java 6 anymore.

Modifications:

Use InetSocketAddress.getHostName() which is practically same and also
is available in Java 6.

Result:

Netty 4.1 runs on Java 6 again.
2015-03-10 11:45:56 +09:00
Trustin Lee
ec097f61b4 Add a new constructor without handler parameter to TrafficCounter
Related: #3476

Motivation:

Some users use TrafficCounter for other uses than we originally
intended, such as implementing their own traffic shaper.  In such a
case, a user does not want to specify an AbstractTrafficShapingHandler.

Modifications:

- Add a new constructor that does not require an
  AbstractTrafficShapingHandler, so that a user can use it without it.
- Simplify TrafficMonitoringTask
- Javadoc cleanup

Result:

We open the possibility of using TrafficCounter for other purposes than
just using it with AbstractTrafficShapingHandler.  Eventually, we could
generalize it a little bit more, so that we can potentially use it for
other uses.
2015-03-10 11:28:41 +09:00
Osvaldo Doederlein
ebfc38cdc5 Fix redundancy in javadocs 2015-03-10 00:08:41 +01:00
Leo Gomes
7d19aef945 Add unit to maxContentLength message javadoc
Motivation:
Not knowing which unit is returned by the maxContentLength() of the Messageggregator when reading the Javadoc is annoying and can be a source of bugs.

Modifications:
Added the mention "in bytes"

Result:
Javadoc is clear.
2015-03-05 20:47:51 +01:00
Leo Gomes
e4df3c694f Add unit to maxContentLength javadoc of HttpObjectAggregator
Motivation:
Not knowing which unit is used for the maxContentLength of the HttpObjectAggregator when reading the Javadoc is annoying and can be a source of bugs.

Modifications:
Added the mention "in bytes"

Result:
Javadoc is clear.
2015-03-05 20:47:51 +01:00
Trustin Lee
e9d1aa2435 Fix SocketException in NioSocketChannelUnsafe.closeExecutor()
Related: #3464

Motivation:

When a connection attempt is failed,
NioSocketChannelUnsafe.closeExecutor() triggers a SocketException,
suppressing the channelUnregistered() event.

Modification:

Do not attempt to get SO_LINGER value when a socket is not open yet.

Result:

One less bug
2015-03-05 15:18:27 +09:00
Norman Maurer
5a0a75fe6f [#3463] EpollSocketChannel.localAddress() returns always null if Native.connect() was not able to connect directly
Motivation:

Due a a regression that was introduced by b898bdd we failed to set the localAddress if the connect did not success directly.

Modifications:

Correct set localAddress in doConnect(...)

Result:

Be able to get the localAddress in all cases.
2015-03-04 20:07:29 +01:00
Trustin Lee
c42ab4bfd1 Fix header and initial line length counting
Related: #3445

Motivation:

HttpObjectDecoder.HeaderParser does not reset its counter (the size
field) when it failed to find the end of line.  If a header is split
into multiple fragments, the counter is increased as many times as the
number of fragments, resulting an unexpected TooLongFrameException.

Modifications:

- Add test cases that reproduces the problem
- Reset the HeaderParser.size field when no EOL is found.

Result:

One less bug
2015-03-04 17:24:45 +09:00
Leo Gomes
f230d59148 Updates the javadoc of Unpooled to remove mention to methods it does not provide
Motivation:

`Unpooled` javadoc's mentioned the generation of hex dump and swapping an integer's byte order,
which are actually provided by `ByteBufUtil`.

Modifications:

 Sentence moved to `ByteBufUtil` javadoc.

Result:

`Unpooled` javadoc is correct.
2015-03-04 12:04:27 +09:00
Norman Maurer
a0ca605425 [maven-release-plugin] prepare for next development iteration 2015-03-03 08:30:59 -05:00